Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable Sveltos Drift Detection #901

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

wahabmk
Copy link
Contributor

@wahabmk wahabmk commented Jan 15, 2025

Description

This PR exposes the drift correction functionality of Sveltos via the ClusterDeployment and MultiClusterService objects by:

  1. Introducing .spec.serviceSpec.syncMode: Sveltos will automatically deploy a drift-detection-manager on the managed cluster by setting this to ContinuousWithDriftDetection.
  2. Introducing .spec.serviceSpec.driftExclusions: To exclude any particular field of an object from drift detection/correction.
  3. Introducing .spec.serviceSpec.ignoreDrift: To exclude any object as a whole from drift detection/correction.

Verification

I created a new managed cluster with the following ClusterDeployment:

apiVersion: k0rdent.mirantis.com/v1alpha1
kind: ClusterDeployment
metadata:
  name: wali-dev-1
  namespace: kcm-system
spec:
  config:
    clusterIdentity:
      name: aws-cluster-identity
      namespace: kcm-system
    controlPlane:
      amiID: ami-0eb9fdcf0d07bd5ef
      instanceProfile: control-plane.cluster-api-provider-aws.sigs.k8s.io
      instanceType: t3.small
    controlPlaneNumber: 1
    publicIP: true
    region: ca-central-1
    worker:
      amiID: ami-0eb9fdcf0d07bd5ef
      instanceProfile: nodes.cluster-api-provider-aws.sigs.k8s.io
      instanceType: t3.small
    workersNumber: 1
  credential: aws-cluster-identity-cred
  serviceSpec:
    services:
      - template: ingress-nginx-4-11-0
        name: ingress-nginx
        namespace: ingress-nginx
        values: |
          ingress-nginx:
            controller:
              replicaCount: 3
    priority: 100
    stopOnConflict: false
    syncMode: ContinuousWithDriftDetection
  template: aws-standalone-cp-0-0-5

Sveltos drift-detection-manager installed on managed cluster

Verified that Sveltos automatically installs the drift-detection-manager on the managed cluster when using syncMode: ContinuousWithDriftDetection:

~ kubectl get pod -A
NAMESPACE        NAME                                        READY   STATUS    RESTARTS   AGE
ingress-nginx    ingress-nginx-controller-86bd747cf9-2gc98   1/1     Running   0          123m
ingress-nginx    ingress-nginx-controller-86bd747cf9-2prlk   1/1     Running   0          123m
ingress-nginx    ingress-nginx-controller-86bd747cf9-bzhb4   1/1     Running   0          123m
kube-system      aws-cloud-controller-manager-cgcnk          1/1     Running   0          124m
kube-system      calico-kube-controllers-6cd7d8cc9f-7vp67    1/1     Running   0          125m
kube-system      calico-node-w22bx                           1/1     Running   0          125m
kube-system      calico-node-z2tsg                           1/1     Running   0          124m
kube-system      coredns-679c655b6f-6mgct                    1/1     Running   0          124m
kube-system      coredns-679c655b6f-qrrc4                    1/1     Running   0          124m
kube-system      ebs-csi-controller-977d5cc56-hztb2          5/5     Running   0          125m
kube-system      ebs-csi-controller-977d5cc56-mj758          5/5     Running   0          125m
kube-system      ebs-csi-node-gj6nj                          3/3     Running   0          124m
kube-system      ebs-csi-node-vqc6n                          3/3     Running   0          125m
kube-system      kube-proxy-kmzcq                            1/1     Running   0          124m
kube-system      kube-proxy-qdkjq                            1/1     Running   0          125m
kube-system      metrics-server-78c4ccbc7f-qxxz4             1/1     Running   0          125m
projectsveltos   drift-detection-manager-6767d5bf67-7dt9x    1/1     Running   0          125m

Verifying that Sveltos corrects drift

I manually edited the ingress-nginx-controller Deployment and changed .spec.replicas=1 to introduce drift. After a few seconds, Sveltos recognized the drift and corrected it as can be seen by the watch on .spec.replicas below. I.e., it starts as 3 then changes to 1 as I manually edit it and then eventually comes back to 3 as Sveltos corrects the drift:

~ kubectl -n ingress-nginx get deployments.apps ingress-nginx-controller -o=jsonpath='{.spec.replicas}' -w
3111333333

We can also see 2 of the pods have a younger age as expected:

~ kubectl -n ingress-nginx get pod   
NAME                                        READY   STATUS    RESTARTS   AGE
ingress-nginx-controller-86bd747cf9-2wbqz   1/1     Running   0          8m43s
ingress-nginx-controller-86bd747cf9-bzhb4   1/1     Running   0          152m
ingress-nginx-controller-86bd747cf9-tkhmh   1/1     Running   0          8m43s

Verifying that driftExclusions work

I used to following drift exclusion in the ClusterDeployment object to exclude .spec.replicas from drift correction:

apiVersion: k0rdent.mirantis.com/v1alpha1
kind: ClusterDeployment
metadata:
  . . .
spec:
  . . .
  serviceSpec:
    services:
      . . .
    priority: 100
    stopOnConflict: false
    syncMode: ContinuousWithDriftDetection
    driftExclusions:
      - paths:
        - "/spec/replicas"
        target:
          kind: Deployment
          name: ingress-nginx-controller
          namespace: ingress-nginx
  . . .

Now when I manually edit the replicas to be 1, the number of replicas is not corrected back to 3 as can be seen below:

~ kubectl -n ingress-nginx get deployments.apps ingress-nginx-controller -o=jsonpath='{.spec.replicas}' -w
3111

We can also verify that this is the case by observing that the "generation" of the ResourceSummary has progressed and by seeing the following patch in its spec:

~ ksveltos get resourcesummaries.lib.projectsveltos.io kcm-system--p--wali-dev-1-capi-wali-dev-1 -o yaml -w
apiVersion: lib.projectsveltos.io/v1beta1
kind: ResourceSummary
metadata:
  . . .
spec:
  chartResources:
  - chartName: ingress-nginx
    group:
    . . .
    releaseName: ingress-nginx
    releaseNamespace: ingress-nginx
  patches:
  - patch: |-
      - op: remove
        path: /spec/replicas
    target:
      kind: Deployment
      name: ingress-nginx-controller
      namespace: ingress-nginx
status:
  helmResourceHashes:
  . . .

Removing the drift exclusion

The drift exclusion can be removed by removing the .spec.serviceSpec.driftExclusion field from the ClusterDeployment object and re-trigger the drift correction by editing any field in the ingress-nginx-controller Deployment. This will force a drift correction and since the drift exclusion has been removed, it will restore the Deployment to it's original spec.

Verifying that ignoreDrift works:

I manually removed the label app.kubernetes.io/managed-by=Helm from the deployment and Sveltos corrected it as expected and as can be seen the watch below:

~ kubectl -n ingress-nginx get deployments.apps ingress-nginx-controller --show-labels -w
NAME                       READY   UP-TO-DATE   AVAILABLE   AGE    LABELS
ingress-nginx-controller   3/3     3            3           171m   app.kubernetes.io/component=controller,app.kubernetes.io/instance=ingress-nginx,app.kubernetes.io/managed-by=Helm,app.kubernetes.io/name=ingress-nginx,app.kubernetes.io/part-of=ingress-nginx,app.kubernetes.io/version=1.11.0,helm.sh/chart=ingress-nginx-4.11.0
ingress-nginx-controller   3/3     3            3           172m   app.kubernetes.io/component=controller,app.kubernetes.io/instance=ingress-nginx,app.kubernetes.io/name=ingress-nginx,app.kubernetes.io/part-of=ingress-nginx,app.kubernetes.io/version=1.11.0,helm.sh/chart=ingress-nginx-4.11.0
ingress-nginx-controller   3/3     3            3           172m   app.kubernetes.io/component=controller,app.kubernetes.io/instance=ingress-nginx,app.kubernetes.io/managed-by=Helm,app.kubernetes.io/name=ingress-nginx,app.kubernetes.io/part-of=ingress-nginx,app.kubernetes.io/version=1.11.0,helm.sh/chart=ingress-nginx-4.11.0

So now let's tell Sveltos to ignore any changes to the ingress-nginx-controller Deployment with the following:

apiVersion: k0rdent.mirantis.com/v1alpha1
kind: ClusterDeployment
metadata:
  . . .
spec:
  . . .
  serviceSpec:
    services:
      . . .
    priority: 100
    stopOnConflict: false
    syncMode: ContinuousWithDriftDetection
    ignoreDrift:
      - target:
        group: apps
        version: v1
        kind: Deployment
        name: ingress-nginx-controller
        namespace: ingress-nginx
  . . .

Now when I manually remove the app.kubernetes.io/managed-by=Helm label again, I can observe that Sveltos does not correct the drift:

~ kubectl -n ingress-nginx get deployments.apps ingress-nginx-controller --show-labels -w
NAME                       READY   UP-TO-DATE   AVAILABLE   AGE     LABELS
ingress-nginx-controller   3/3     3            3           3h58m   app.kubernetes.io/component=controller,app.kubernetes.io/instance=ingress-nginx,app.kubernetes.io/managed-by=Helm,app.kubernetes.io/name=ingress-nginx,app.kubernetes.io/part-of=ingress-nginx,app.kubernetes.io/version=1.11.0,helm.sh/chart=ingress-nginx-4.11.0
ingress-nginx-controller   3/3     3            3           3h59m   app.kubernetes.io/component=controller,app.kubernetes.io/instance=ingress-nginx,app.kubernetes.io/name=ingress-nginx,app.kubernetes.io/part-of=ingress-nginx,app.kubernetes.io/version=1.11.0,helm.sh/chart=ingress-nginx-4.11.0

This can also be verified by observing that ignoreForConfigurationDrift: true for the object targeted for drift ignore in ResourceSummary spec on the managed cluser:

~ ksveltos get resourcesummaries.lib.projectsveltos.io kcm-system--p--wali-dev-1-capi-wali-dev-1 -o yaml -w
apiVersion: lib.projectsveltos.io/v1beta1
kind: ResourceSummary
metadata:
  . . .
spec:
  chartResources:
  - chartName: ingress-nginx
    group:
    . . .
    - group: apps
      ignoreForConfigurationDrift: true
      kind: Deployment
      name: ingress-nginx-controller
      namespace: ingress-nginx
      version: v1
    releaseName: ingress-nginx
    releaseNamespace: ingress-nginx
status:
  helmResourceHashes:
  . . .

And also by observing that the projectsveltos.io/driftDetectionIgnore: ok annotation has been applied to the targeted object:

~ kubectl -n ingress-nginx get deployments.apps ingress-nginx-controller -o=jsonpath='{.metadata.annotations}'
{"deployment.kubernetes.io/revision":"1","meta.helm.sh/release-name":"ingress-nginx","meta.helm.sh/release-namespace":"ingress-nginx","projectsveltos.io/driftDetectionIgnore":"ok"}%

Removing the ignore drift:

The ignore drift setting can be removed by removing the .spec.serviceSpec.ignoreDrift field from the ClusterDeployment object, Sveltos will then automatically re-trigger the drift correction.

@wahabmk wahabmk self-assigned this Jan 15, 2025
@wahabmk wahabmk force-pushed the support-drift-detection branch 3 times, most recently from c449e49 to 96142d3 Compare January 20, 2025 16:22
@wahabmk wahabmk linked an issue Jan 20, 2025 that may be closed by this pull request
@wahabmk wahabmk force-pushed the support-drift-detection branch 2 times, most recently from 8cec7cf to 6c94991 Compare January 21, 2025 10:13
@wahabmk wahabmk marked this pull request as ready for review January 21, 2025 10:45
@wahabmk wahabmk force-pushed the support-drift-detection branch from 6c94991 to 113c322 Compare January 23, 2025 12:30
Copy link
Contributor

@BROngineer BROngineer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

// +kubebuilder:validation:Enum:=OneTime;Continuous;ContinuousWithDriftDetection;DryRun

SyncMode string `json:"syncMode,omitempty"`
IgnoreDrift []libsveltosv1beta1.PatchSelector `json:"ignoreDrift,omitempty"`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a small thing but should we rename this to DriftIgnore to make it similar to DriftExclusions? 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Allow for users to set config for Sveltos Drift Detection
2 participants