Skip to content

Commit

Permalink
feat(dp): handle PodDisruptionBudget reconciliation for deployment
Browse files Browse the repository at this point in the history
  • Loading branch information
czeslavo committed Aug 7, 2024
1 parent 76289d6 commit 8baa7e5
Show file tree
Hide file tree
Showing 22 changed files with 419 additions and 40 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@
- Introduce `KongPluginInstallation` CRD to allow installing custom Kong
plugins distributed as container images.
[400](https://github.com/Kong/gateway-operator/pull/400)
- Extended `DataPlane` API with a possibility to specify `PodDisruptionBudget` to be
created for the `DataPlane` deployments via `spec.resources.podDisruptionBudget`.
[#464](https://github.com/Kong/gateway-operator/pull/464)

### Fixed

Expand Down
2 changes: 1 addition & 1 deletion api/v1beta1/dataplane_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ type DataPlaneStatus struct {

// Selector contains a unique DataPlane identifier used as a deterministic
// label selector that is used throughout its dependent resources.
// This is used e.g. as a label selector for DataPlane's Services and Deployments.
// This is used e.g. as a label selector for DataPlane's Services, Deployments and PodDisruptionBudgets.
//
// +kubebuilder:validation:MaxLength=512
// +kubebuilder:validation:MinLength=8
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9322,7 +9322,7 @@ spec:
description: |-
Selector contains a unique DataPlane identifier used as a deterministic
label selector that is used throughout its dependent resources.
This is used e.g. as a label selector for DataPlane's Services and Deployments.
This is used e.g. as a label selector for DataPlane's Services, Deployments and PodDisruptionBudgets.
maxLength: 512
minLength: 8
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9322,7 +9322,7 @@ spec:
description: |-
Selector contains a unique DataPlane identifier used as a deterministic
label selector that is used throughout its dependent resources.
This is used e.g. as a label selector for DataPlane's Services and Deployments.
This is used e.g. as a label selector for DataPlane's Services, Deployments and PodDisruptionBudgets.
maxLength: 512
minLength: 8
type: string
Expand Down
17 changes: 17 additions & 0 deletions config/rbac/role/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,23 @@ rules:
- patch
- update
- watch
- apiGroups:
- policy
resources:
- poddisruptionbudgets
verbs:
- create
- get
- list
- patch
- update
- watch
- apiGroups:
- policy
resources:
- poddistruptionbudgets
verbs:
- delete
- apiGroups:
- rbac.authorization.k8s.io
resources:
Expand Down
9 changes: 9 additions & 0 deletions controller/dataplane/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,15 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
return ctrl.Result{}, nil
}

res, _, err = ensurePodDisruptionBudgetForDataPlane(ctx, r.Client, logger, dataplane)
if err != nil {
return ctrl.Result{}, fmt.Errorf("could not ensure PodDisruptionBudget for DataPlane %s/%s: %w", dataplane.Namespace, dataplane.Name, err)
}
if res != op.Noop {
log.Debug(logger, "PodDisruptionBudget created/updated", dataplane)
return ctrl.Result{}, nil
}

if res, err := ensureDataPlaneReadyStatus(ctx, r.Client, logger, dataplane, dataplane.Generation); err != nil {
return ctrl.Result{}, err
} else if res.Requeue {
Expand Down
23 changes: 12 additions & 11 deletions controller/dataplane/controller_rbac.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@ package dataplane
// DataPlaneReconciler - RBAC
// -----------------------------------------------------------------------------

//+kubebuilder:rbac:groups=gateway-operator.konghq.com,resources=dataplanes,verbs=get;list;watch;update;patch
//+kubebuilder:rbac:groups=gateway-operator.konghq.com,resources=dataplanes/status,verbs=get;update;patch
//+kubebuilder:rbac:groups=gateway-operator.konghq.com,resources=dataplanes/finalizers,verbs=update
//+kubebuilder:rbac:groups=apps,resources=deployments,verbs=create;get;list;watch;update;patch;delete
//+kubebuilder:rbac:groups=apps,resources=deployments/status,verbs=get
//+kubebuilder:rbac:groups=core,resources=services,verbs=create;get;list;watch;update;patch;delete
//+kubebuilder:rbac:groups=core,resources=services/status,verbs=get
//+kubebuilder:rbac:groups=core,resources=configmaps,verbs=get;list;watch
//+kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch;delete
//+kubebuilder:rbac:groups="",resources=events,verbs=create;patch
//+kubebuilder:rbac:groups=autoscaling,resources=horizontalpodautoscalers,verbs=create;get;list;patch;watch
// +kubebuilder:rbac:groups=gateway-operator.konghq.com,resources=dataplanes,verbs=get;list;watch;update;patch
// +kubebuilder:rbac:groups=gateway-operator.konghq.com,resources=dataplanes/status,verbs=get;update;patch
// +kubebuilder:rbac:groups=gateway-operator.konghq.com,resources=dataplanes/finalizers,verbs=update
// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=create;get;list;watch;update;patch;delete
// +kubebuilder:rbac:groups=apps,resources=deployments/status,verbs=get
// +kubebuilder:rbac:groups=core,resources=services,verbs=create;get;list;watch;update;patch;delete
// +kubebuilder:rbac:groups=core,resources=services/status,verbs=get
// +kubebuilder:rbac:groups=core,resources=configmaps,verbs=get;list;watch
// +kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch;delete
// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch
// +kubebuilder:rbac:groups=autoscaling,resources=horizontalpodautoscalers,verbs=create;get;list;patch;watch
// +kubebuilder:rbac:groups=policy,resources=poddisruptionbudgets,verbs=create;get;list;watch;update;patch
56 changes: 56 additions & 0 deletions controller/dataplane/owned_resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
autoscalingv2 "k8s.io/api/autoscaling/v2"
certificatesv1 "k8s.io/api/certificates/v1"
corev1 "k8s.io/api/core/v1"
policyv1 "k8s.io/api/policy/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/selection"
Expand Down Expand Up @@ -113,6 +114,61 @@ func ensureHPAForDataPlane(
return op.Created, nil, nil
}

func ensurePodDisruptionBudgetForDataPlane(
ctx context.Context,
cl client.Client,
log logr.Logger,
dataplane *operatorv1beta1.DataPlane,
) (res op.Result, pdb *policyv1.PodDisruptionBudget, err error) {
matchingLabels := k8sresources.GetManagedLabelForOwner(dataplane)
pdbs, err := k8sutils.ListPodDisruptionBudgetsForOwner(ctx, cl, dataplane.Namespace, dataplane.UID, matchingLabels)
if err != nil {
return op.Noop, nil, fmt.Errorf("failed listing PodDisruptionBudgets for DataPlane %s/%s: %w", dataplane.Namespace, dataplane.Name, err)
}

if dataplane.Spec.Resources.PodDisruptionBudget == nil {
if err := k8sreduce.ReducePodDisruptionBudgets(ctx, cl, pdbs, k8sreduce.FilterNone); err != nil {
return op.Noop, nil, fmt.Errorf("failed reducing PodDisruptionBudgets for DataPlane %s/%s: %w", dataplane.Namespace, dataplane.Name, err)
}
return op.Noop, nil, nil
}

if len(pdbs) > 1 {
if err := k8sreduce.ReducePodDisruptionBudgets(ctx, cl, pdbs, k8sreduce.FilterPodDisruptionBudgets); err != nil {
return op.Noop, nil, fmt.Errorf("failed reducing PodDisruptionBudgets for DataPlane %s/%s: %w", dataplane.Namespace, dataplane.Name, err)
}
return op.Noop, nil, nil
}

generatedPDB, err := k8sresources.GeneratePodDisruptionBudgetForDataPlane(dataplane)
if err != nil {
return op.Noop, nil, fmt.Errorf("failed generating PodDisruptionBudget for DataPlane %s/%s: %w", dataplane.Namespace, dataplane.Name, err)
}

if len(pdbs) == 1 {
var updated bool
existingPDB := &pdbs[0]
oldExistingPDB := existingPDB.DeepCopy()

// Ensure that PDB's metadata is up-to-date.
updated, existingPDB.ObjectMeta = k8sutils.EnsureObjectMetaIsUpdated(existingPDB.ObjectMeta, generatedPDB.ObjectMeta)

// Ensure that PDB's spec is up-to-date.
if !cmp.Equal(existingPDB.Spec, generatedPDB.Spec) {
existingPDB.Spec = generatedPDB.Spec
updated = true
}

return patch.ApplyPatchIfNonEmpty(ctx, cl, log, existingPDB, oldExistingPDB, dataplane, updated)
}

if err := cl.Create(ctx, generatedPDB); err != nil {
return op.Noop, nil, fmt.Errorf("failed creating PodDisruptionBudget for DataPlane %s: %w", dataplane.Name, err)
}

return op.Created, generatedPDB, nil
}

func matchingLabelsToServiceOpt(ml client.MatchingLabels) k8sresources.ServiceOpt {
return func(s *corev1.Service) {
if s.Labels == nil {
Expand Down
15 changes: 9 additions & 6 deletions controller/dataplane/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
appsv1 "k8s.io/api/apps/v1"
autoscalingv2 "k8s.io/api/autoscaling/v2"
corev1 "k8s.io/api/core/v1"
policyv1 "k8s.io/api/policy/v1"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"

Expand All @@ -15,14 +16,16 @@ import (
// the operator.
func DataPlaneWatchBuilder(mgr ctrl.Manager) *builder.Builder {
return ctrl.NewControllerManagedBy(mgr).
// watch DataPlane objects
// Watch DataPlane objects.
For(&operatorv1beta1.DataPlane{}).
// watch for changes in Secrets created by the dataplane controller
// Watch for changes in Secrets created by the dataplane controller.
Owns(&corev1.Secret{}).
// watch for changes in Services created by the dataplane controller
// Watch for changes in Services created by the dataplane controller.
Owns(&corev1.Service{}).
// watch for changes in Deployments created by the dataplane controller
// Watch for changes in Deployments created by the dataplane controller.
Owns(&appsv1.Deployment{}).
// watch for changes in HPA created by the dataplane controller
Owns(&autoscalingv2.HorizontalPodAutoscaler{})
// Watch for changes in HPA created by the dataplane controller.
Owns(&autoscalingv2.HorizontalPodAutoscaler{}).
// Watch for changes in PodDisruptionBudgets created by the dataplane controller.
Owns(&policyv1.PodDisruptionBudget{})
}
7 changes: 2 additions & 5 deletions controller/konnect/constraints.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,15 @@ type SupportedKonnectEntityType interface {
// Separating this from SupportedKonnectEntityType allows us to use EntityType
// where client.Object is required, since it embeds client.Object and uses pointer
// to refer to the SupportedKonnectEntityType.
type EntityType[
T SupportedKonnectEntityType,
] interface {
type EntityType[T SupportedKonnectEntityType] interface {
*T

// Kubernetes Object methods
GetObjectMeta() metav1.Object
client.Object

// Additional methods which are used in reconciling Konnect entities.
GetConditions() []metav1.Condition
SetConditions([]metav1.Condition)
GetKonnectStatus() *configurationv1alpha1.KonnectEntityStatus
GetKonnectStatus() *konnectv1alpha1.KonnectEntityStatus
GetKonnectAPIAuthConfigurationRef() konnectv1alpha1.KonnectAPIAuthConfigurationRef
}
3 changes: 2 additions & 1 deletion controller/pkg/patch/patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/go-logr/logr"
appsv1 "k8s.io/api/apps/v1"
autoscalingv2 "k8s.io/api/autoscaling/v2"
policyv1 "k8s.io/api/policy/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"

Expand All @@ -23,7 +24,7 @@ import (
func ApplyPatchIfNonEmpty[
OwnerT *operatorv1beta1.DataPlane | *operatorv1beta1.ControlPlane,
ResourceT interface {
*appsv1.Deployment | *autoscalingv2.HorizontalPodAutoscaler | *certmanagerv1.Certificate
*appsv1.Deployment | *autoscalingv2.HorizontalPodAutoscaler | *certmanagerv1.Certificate | *policyv1.PodDisruptionBudget
client.Object
},
](
Expand Down
2 changes: 1 addition & 1 deletion docs/api-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -894,7 +894,7 @@ DataPlaneStatus defines the observed state of DataPlane
| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#condition-v1-meta) array_ | Conditions describe the status of the DataPlane. |
| `service` _string_ | Service indicates the Service that exposes the DataPlane's configured routes |
| `addresses` _[Address](#address) array_ | Addresses lists the addresses that have actually been bound to the DataPlane. |
| `selector` _string_ | Selector contains a unique DataPlane identifier used as a deterministic label selector that is used throughout its dependent resources. This is used e.g. as a label selector for DataPlane's Services and Deployments. |
| `selector` _string_ | Selector contains a unique DataPlane identifier used as a deterministic label selector that is used throughout its dependent resources. This is used e.g. as a label selector for DataPlane's Services, Deployments and PodDisruptionBudgets. |
| `readyReplicas` _integer_ | ReadyReplicas indicates how many replicas have reported to be ready. |
| `replicas` _integer_ | Replicas indicates how many replicas have been set for the DataPlane. |
| `rollout` _[DataPlaneRolloutStatus](#dataplanerolloutstatus)_ | RolloutStatus contains information about the rollout. It is set only if a rollout strategy was configured in the spec. |
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ require (
github.com/cloudflare/cfssl v1.6.5
github.com/go-logr/logr v1.4.2
github.com/google/uuid v1.6.0
github.com/kong/kubernetes-configuration v0.0.0-20240801170722-d6586edd1b81
github.com/kong/kubernetes-configuration v0.0.1
github.com/kong/kubernetes-ingress-controller/v3 v3.2.3
github.com/kong/kubernetes-telemetry v0.1.4
github.com/kong/kubernetes-testing-framework v0.47.1
Expand Down
14 changes: 2 additions & 12 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1/go.mod h1:xomTg6
github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
github.com/BurntSushi/toml v1.3.2 h1:o7IhLm0Msx3BaB+n3Ag7L8EVlByGnpq14C4YWiu/gL8=
github.com/BurntSushi/toml v1.3.2/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ=
github.com/Kong/sdk-konnect-go v0.0.0-20240723160412-999d9a987e1a h1:0mQhPVVA2/+uTVmoKrEIGf+0eTrNyr80Ssv1zGs/1Lk=
github.com/Kong/sdk-konnect-go v0.0.0-20240723160412-999d9a987e1a/go.mod h1:ipu67aQNnwDzu/LXKePG46cVqkkZnAHKWpsbhTEI8xE=
github.com/Kong/sdk-konnect-go v0.0.0-20240801091928-39a27951b473 h1:vIYHnHxEcWTGPZ113NPlLvWEOWYaPRYwwpuoU2J64yY=
github.com/Kong/sdk-konnect-go v0.0.0-20240801091928-39a27951b473/go.mod h1:75YzLhfnYfmCvBJgkafzVuREwBAec2/jihCW2fyn6hY=
github.com/Masterminds/semver v1.5.0 h1:H65muMkzWKEuNDnfl9d70GUjFniHKHRbFPGBuZ3QEww=
Expand Down Expand Up @@ -219,16 +217,8 @@ github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI
github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
github.com/kong/go-kong v0.57.0 h1:e+4bHTzcO0xhFPVyGIUtlO+B4E4l14k55NH8Vjw6ORY=
github.com/kong/go-kong v0.57.0/go.mod h1:gyNwyP1fzztT6sX/0/ygMQ30OiRMIQ51b2jSfstMrcU=
github.com/kong/kubernetes-configuration v0.0.0-20240726122834-c10e8297f9e6 h1:mi0P9KZSYbI1bffC+dwz0lesjHgd3AS4W/762C6qJHs=
github.com/kong/kubernetes-configuration v0.0.0-20240726122834-c10e8297f9e6/go.mod h1:kZTKzwQ68Wk2n8W8Em0RsYTL2yVNbCWU+5b9w1WU+Hs=
github.com/kong/kubernetes-configuration v0.0.0-20240731182605-13d36c4c628c h1:D+JNLhsjs3I9R4tUp9/0EknDG2DayVny20rEGj33o/w=
github.com/kong/kubernetes-configuration v0.0.0-20240731182605-13d36c4c628c/go.mod h1:kZTKzwQ68Wk2n8W8Em0RsYTL2yVNbCWU+5b9w1WU+Hs=
github.com/kong/kubernetes-configuration v0.0.0-20240801095618-0ed30e4054cb h1:GUMgT3qqNjLV1Wdl8D6fskEaN/vof/FRhRbicIkUKpg=
github.com/kong/kubernetes-configuration v0.0.0-20240801095618-0ed30e4054cb/go.mod h1:kZTKzwQ68Wk2n8W8Em0RsYTL2yVNbCWU+5b9w1WU+Hs=
github.com/kong/kubernetes-configuration v0.0.0-20240801155137-c98bf64df469 h1:wzRoAf4byqST4BvmqPne/GL56VrnsrRM3A8ePuvOyf4=
github.com/kong/kubernetes-configuration v0.0.0-20240801155137-c98bf64df469/go.mod h1:kZTKzwQ68Wk2n8W8Em0RsYTL2yVNbCWU+5b9w1WU+Hs=
github.com/kong/kubernetes-configuration v0.0.0-20240801170722-d6586edd1b81 h1:yEbqo7RdTQ7iGEA1PIpQGfO7D5EN7jj2qdPOGN9ipow=
github.com/kong/kubernetes-configuration v0.0.0-20240801170722-d6586edd1b81/go.mod h1:kZTKzwQ68Wk2n8W8Em0RsYTL2yVNbCWU+5b9w1WU+Hs=
github.com/kong/kubernetes-configuration v0.0.1 h1:JbeqN/B5FADzxHBWy1IDvkIvj6xbiEKBefDV47yKaSQ=
github.com/kong/kubernetes-configuration v0.0.1/go.mod h1:kZTKzwQ68Wk2n8W8Em0RsYTL2yVNbCWU+5b9w1WU+Hs=
github.com/kong/kubernetes-ingress-controller/v3 v3.2.3 h1:SQ/0hfceGmsvzbkCUxiJUv1ELcFRp4d6IzvYGfHct9o=
github.com/kong/kubernetes-ingress-controller/v3 v3.2.3/go.mod h1:gshVZnDU2FTe/95I3vSJPsH2kyB8zR+GpUIieCyt8C4=
github.com/kong/kubernetes-telemetry v0.1.4 h1:Yz7OlECxWKgNRG1wJ5imA4+H0dQEpdU9d86uhwUVpu4=
Expand Down
36 changes: 36 additions & 0 deletions pkg/utils/kubernetes/lists.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
appsv1 "k8s.io/api/apps/v1"
autoscalingv2 "k8s.io/api/autoscaling/v2"
corev1 "k8s.io/api/core/v1"
policyv1 "k8s.io/api/policy/v1"
rbacv1 "k8s.io/api/rbac/v1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -83,6 +84,41 @@ func ListHPAsForOwner(
return hpas, nil
}

// ListPodDisruptionBudgetsForOwner is a helper function which gets a list of PodDisruptionBudget
// using the provided list options and reduce by OwnerReference UID and namespace to efficiently
// list only the objects owned by the provided UID.
func ListPodDisruptionBudgetsForOwner(
ctx context.Context,
c client.Client,
namespace string,
uid types.UID,
listOpts ...client.ListOption,
) ([]policyv1.PodDisruptionBudget, error) {
pdbList := &policyv1.PodDisruptionBudgetList{}

err := c.List(
ctx,
pdbList,
append(
[]client.ListOption{client.InNamespace(namespace)},
listOpts...,
)...,
)
if err != nil {
return nil, err
}

var pdbs []policyv1.PodDisruptionBudget
for _, pdb := range pdbList.Items {
pdb := pdb
if IsOwnedByRefUID(&pdb, uid) {
pdbs = append(pdbs, pdb)
}
}

return pdbs, nil
}

// ListServicesForOwner is a helper function which gets a list of Services
// using the provided list options and reduce by OwnerReference UID and namespace to efficiently
// list only the objects owned by the provided UID.
Expand Down
24 changes: 24 additions & 0 deletions pkg/utils/kubernetes/reduce/filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
corev1 "k8s.io/api/core/v1"
discoveryv1 "k8s.io/api/discovery/v1"
networkingv1 "k8s.io/api/networking/v1"
policyv1 "k8s.io/api/policy/v1"
rbacv1 "k8s.io/api/rbac/v1"

operatorv1beta1 "github.com/kong/gateway-operator/api/v1beta1"
Expand Down Expand Up @@ -367,6 +368,29 @@ func FilterHPAs(hpas []autoscalingv2.HorizontalPodAutoscaler) []autoscalingv2.Ho
return append(hpas[:best], hpas[best+1:]...)
}

// -----------------------------------------------------------------------------
// Filter functions - PodDisruptionBudgets
// -----------------------------------------------------------------------------

// FilterPodDisruptionBudgets filters out the PodDisruptionBudgets to be kept and returns all
// the PodDisruptionBudgets to be deleted.
// The filtered-out PodDisruptionBudget is decided as follows:
// 1. creationTimestamp (older is better)
func FilterPodDisruptionBudgets(pdbs []policyv1.PodDisruptionBudget) []policyv1.PodDisruptionBudget {
if len(pdbs) < 2 {
return nil
}

best := 0
for i, hpa := range pdbs {
if hpa.CreationTimestamp.Before(&pdbs[best].CreationTimestamp) {
best = i
}
}

return append(pdbs[:best], pdbs[best+1:]...)
}

// -----------------------------------------------------------------------------
// Filter functions - ValidatingWebhookConfigurations
// -----------------------------------------------------------------------------
Expand Down
Loading

0 comments on commit 8baa7e5

Please sign in to comment.