Stop argo pruning skiperator owned resources #160

eliihen · 2023-02-14T09:22:00Z

When Argo and Skiperator run in the same system they conflict when tearing down a namespace. This PR tells Argo not to touch skiperator app generated resources so namespaces don't get into an infinite terminating state.

Uses this annotation: https://argo-cd.readthedocs.io/en/stable/user-guide/sync-options/#no-prune-resources

Tested in sandbox. Seems to work.

github-actions · 2023-02-14T09:27:57Z

Results for sandbox – ❗ `CHANGED` ❗

Terraform Format and Style 🖌 `success`

Terraform Initialization ⚙️ `success`

Terraform Validation 🤖 `success`

Validation Output


Success! The configuration is valid.

Terraform Plan 📖 `success`

Show Plan


[command]/home/runner/work/_temp/4fc11e5f-4b8d-4b76-8d98-9d5d35568763/terraform-bin plan -input=false -no-color -detailed-exitcode -out=plan-sandbox.tfplan -var=image=ghcr.io/kartverket/skiperator-controller:sha-ae932773de20c1a4bb5d450de63700da501a04c0 -var-file=sandbox.tfvars
Acquiring state lock. This may take a few moments...

Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # kubernetes_deployment_v1.deployment will be updated in-place
  ~ resource "kubernetes_deployment_v1" "deployment" {
        id               = "skiperator-system/skiperator"
        # (1 unchanged attribute hidden)

      ~ spec {
          ~ replicas                  = "2" -> "3"
            # (4 unchanged attributes hidden)

          ~ template {

              ~ spec {
                    # (12 unchanged attributes hidden)

                  ~ container {
                      ~ image                      = "ghcr.io/kartverket/skiperator-controller:sha-52fb2bc0ce5daf7fdd3de7ec6b049841b379fa1a" -> "ghcr.io/kartverket/skiperator-controller:sha-ae932773de20c1a4bb5d450de63700da501a04c0"
                        name                       = "skiperator"
                        # (8 unchanged attributes hidden)

                        # (7 unchanged blocks hidden)
                    }
                }

                # (1 unchanged block hidden)
            }

            # (2 unchanged blocks hidden)
        }

        # (1 unchanged block hidden)
    }

  # kubernetes_manifest.cluster_role will be updated in-place
  ~ resource "kubernetes_manifest" "cluster_role" {
      ~ object   = {
          ~ metadata        = {
              ~ creationTimestamp          = null -> (known after apply)
                name                       = "skiperator"
                # (13 unchanged elements hidden)
            }
            # (4 unchanged elements hidden)
        }
        # (1 unchanged attribute hidden)

        # (1 unchanged block hidden)
    }

  # kubernetes_manifest.custom_resource_definition will be updated in-place
  ~ resource "kubernetes_manifest" "custom_resource_definition" {
      ~ object   = {
          ~ metadata   = {
              ~ creationTimestamp          = null -> (known after apply)
                name                       = "applications.skiperator.kartverket.no"
                # (13 unchanged elements hidden)
            }
            # (3 unchanged elements hidden)
        }
        # (1 unchanged attribute hidden)

        # (1 unchanged block hidden)
    }

Plan: 0 to add, 3 to change, 0 to destroy.

Warning: "default_secret_name" is no longer applicable for Kubernetes v1.24.0 and above

  with kubernetes_service_account_v1.service_account,
  on role.tf line 1, in resource "kubernetes_service_account_v1" "service_account":
   1: resource "kubernetes_service_account_v1" "service_account" {

Starting from version 1.24.0 Kubernetes does not automatically generate a
token for service accounts, in this case, "default_secret_name" will be empty
Releasing state lock. This may take a few moments...

Next action 🚀

Changes detected. Will run Terraform apply job on merge to

Pusher: @esphen, Working Directory: deployment, Commit: ae93277, Generated at: 15.2.2023, 09:51:57

Results for dev – ❗ `CHANGED` ❗

Terraform Format and Style 🖌 `success`

Terraform Initialization ⚙️ `success`

Terraform Validation 🤖 `success`

Validation Output


Success! The configuration is valid.

Terraform Plan 📖 `success`

Show Plan


[command]/home/runner/work/_temp/d5c0c938-2493-4cfb-b23a-683eea6620e7/terraform-bin plan -input=false -no-color -detailed-exitcode -out=plan-dev.tfplan -var=image=ghcr.io/kartverket/skiperator-controller:sha-ae932773de20c1a4bb5d450de63700da501a04c0 -var-file=dev.tfvars
Acquiring state lock. This may take a few moments...

Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # kubernetes_deployment_v1.deployment will be updated in-place
  ~ resource "kubernetes_deployment_v1" "deployment" {
        id               = "skiperator-system/skiperator"
        # (1 unchanged attribute hidden)

      ~ spec {
            # (5 unchanged attributes hidden)

          ~ template {

              ~ spec {
                    # (12 unchanged attributes hidden)

                  ~ container {
                      ~ image                      = "ghcr.io/kartverket/skiperator-controller:sha-6ddbc4f2bab1f1f967d242ad2c6d60106bdb15dd" -> "ghcr.io/kartverket/skiperator-controller:sha-ae932773de20c1a4bb5d450de63700da501a04c0"
                        name                       = "skiperator"
                        # (8 unchanged attributes hidden)

                        # (7 unchanged blocks hidden)
                    }
                }

                # (1 unchanged block hidden)
            }

            # (2 unchanged blocks hidden)
        }

        # (1 unchanged block hidden)
    }

  # kubernetes_manifest.cluster_role will be updated in-place
  ~ resource "kubernetes_manifest" "cluster_role" {
      ~ object   = {
          ~ metadata        = {
              ~ creationTimestamp          = null -> (known after apply)
                name                       = "skiperator"
                # (14 unchanged elements hidden)
            }
            # (4 unchanged elements hidden)
        }
        # (1 unchanged attribute hidden)

        # (1 unchanged block hidden)
    }

  # kubernetes_manifest.custom_resource_definition will be updated in-place
  ~ resource "kubernetes_manifest" "custom_resource_definition" {
      ~ object   = {
          ~ metadata   = {
              ~ creationTimestamp          = null -> (known after apply)
                name                       = "applications.skiperator.kartverket.no"
                # (14 unchanged elements hidden)
            }
            # (3 unchanged elements hidden)
        }
        # (1 unchanged attribute hidden)

        # (1 unchanged block hidden)
    }

Plan: 0 to add, 3 to change, 0 to destroy.

Warning: "default_secret_name" is no longer applicable for Kubernetes v1.24.0 and above

  with kubernetes_service_account_v1.service_account,
  on role.tf line 1, in resource "kubernetes_service_account_v1" "service_account":
   1: resource "kubernetes_service_account_v1" "service_account" {

Starting from version 1.24.0 Kubernetes does not automatically generate a
token for service accounts, in this case, "default_secret_name" will be empty
Releasing state lock. This may take a few moments...

Next action 🚀

Changes detected. Will run Terraform apply job on merge to

Pusher: @esphen, Working Directory: deployment, Commit: ae93277, Generated at: 15.2.2023, 09:54:11

Results for test

Terraform Format and Style 🖌 `success`

Terraform Initialization ⚙️ ``

Terraform Validation 🤖 `success`

Validation Output


Success! The configuration is valid.

Terraform Plan 📖 ``

Show Plan

Next action 🚀

Terraform gave an unknown exit code, I don't know what happens next!

Pusher: @esphen, Working Directory: deployment, Commit: ae93277, Generated at: 15.2.2023, 11:41:46

Results for prod – ❗ `CHANGED` ❗

Terraform Format and Style 🖌 `success`

Terraform Initialization ⚙️ `success`

Terraform Validation 🤖 `success`

Validation Output


Success! The configuration is valid.

Terraform Plan 📖 `success`

Show Plan


[command]/home/runner/work/_temp/df55dae6-0472-4911-b638-270c2541b077/terraform-bin plan -input=false -no-color -detailed-exitcode -out=plan-prod.tfplan -var=image=ghcr.io/kartverket/skiperator-controller:sha-52fb2bc0ce5daf7fdd3de7ec6b049841b379fa1a -var-file=prod.tfvars
Acquiring state lock. This may take a few moments...

Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # kubernetes_deployment_v1.deployment will be updated in-place
  ~ resource "kubernetes_deployment_v1" "deployment" {
        id               = "skiperator-system/skiperator"
        # (1 unchanged attribute hidden)

      ~ spec {
            # (5 unchanged attributes hidden)

          ~ template {

              ~ spec {
                    # (12 unchanged attributes hidden)

                  ~ container {
                      ~ image                      = "ghcr.io/kartverket/skiperator-controller:sha-56e77751ef9119ce465e6a219f2e52bdbdc5df79" -> "ghcr.io/kartverket/skiperator-controller:sha-52fb2bc0ce5daf7fdd3de7ec6b049841b379fa1a"
                        name                       = "skiperator"
                        # (8 unchanged attributes hidden)

                        # (7 unchanged blocks hidden)
                    }
                }

                # (1 unchanged block hidden)
            }

            # (2 unchanged blocks hidden)
        }

        # (1 unchanged block hidden)
    }

  # kubernetes_manifest.cluster_role will be updated in-place
  ~ resource "kubernetes_manifest" "cluster_role" {
      ~ object   = {
          ~ metadata        = {
              ~ creationTimestamp          = null -> (known after apply)
                name                       = "skiperator"
                # (14 unchanged elements hidden)
            }
            # (4 unchanged elements hidden)
        }
        # (1 unchanged attribute hidden)

        # (1 unchanged block hidden)
    }

  # kubernetes_manifest.custom_resource_definition will be updated in-place
  ~ resource "kubernetes_manifest" "custom_resource_definition" {
      ~ object   = {
          ~ metadata   = {
              ~ creationTimestamp          = null -> (known after apply)
                name                       = "applications.skiperator.kartverket.no"
                # (14 unchanged elements hidden)
            }
            # (3 unchanged elements hidden)
        }
        # (1 unchanged attribute hidden)

        # (1 unchanged block hidden)
    }

Plan: 0 to add, 3 to change, 0 to destroy.

Warning: "default_secret_name" is no longer applicable for Kubernetes v1.24.0 and above

  with kubernetes_service_account_v1.service_account,
  on role.tf line 1, in resource "kubernetes_service_account_v1" "service_account":
   1: resource "kubernetes_service_account_v1" "service_account" {

Starting from version 1.24.0 Kubernetes does not automatically generate a
token for service accounts, in this case, "default_secret_name" will be empty
Releasing state lock. This may take a few moments...

Next action 🚀

Changes detected. Will run Terraform apply job on merge to

Pusher: @esphen, Working Directory: deployment, Commit: 52fb2bc, Generated at: 14.2.2023, 12:07:23

evenh

LGTM, added a suggestion but feel free to ignore :-)

evenh · 2023-02-14T13:14:12Z

controllers/application/deployment.go

+		deployment.Spec.Template.ObjectMeta.Annotations = map[string]string{
+			"argocd.argoproj.io/sync-options": "Prune=false",
+			"prometheus.io/scrape":            "true",
+		}


Since this might be common pattern going forward (common annotation + one or more special cases) I propose the following

Suggested change

deployment.Spec.Template.ObjectMeta.Annotations = map[string]string{

"argocd.argoproj.io/sync-options": "Prune=false",

"prometheus.io/scrape": "true",

}

deployment.Spec.Template.ObjectMeta.Annotations = util.CommonAnnotationsPlus(

"prometheus.io/scrape", "true",

)

And adding the following function in pkg/util/helperfunctions.go:

func CommonAnnotationsPlus(kvs ...string) map[string]string { if len(kvs)%2 != 0 { panic(fmt.Sprintf("both key and value must be supplied, got %s", kvs)) } m := CommonAnnotations for i := 0; i < len(kvs); i = i + 2 { m[kvs[i]] = kvs[i+1] } return m }

omaen

Is it possible to do it the other way around and set a Skiperator specific-annotation on these resources, and tell Argo to ignore them? Just seems a bit intrusive from Skiperators point of view to have to set this annotation.

I have no idea if that's possible, just a thought. Otherwise this looks like a good solution to the problem :)

eliihen · 2023-02-15T07:16:59Z

Is it possible to do it the other way around and set a Skiperator specific-annotation on these resources, and tell Argo to ignore them? Just seems a bit intrusive from Skiperators point of view to have to set this annotation.

@omaen Not sure exactly what you mean, the annotation that is set here makes Argo ignore these resources. Otherwise Argo will try to delete them when terminating a namespace which causes a deadlock when skiperator's finalizers try to delete them again. It's pretty much the same as setting the prometheus/scrape annotation on behalf of the teams as we already do.

It would probably be even more intrusive if we were to do it the other way around and add some feature that made it possible for skiperator to create, but not delete resources so that argo would do deletions. Right now that is done implicitly through the ownership pattern, which tells skiperator what it owns so that it automatically cleans it up afterwards.

omaen · 2023-02-15T08:33:17Z

Is it possible to do it the other way around and set a Skiperator specific-annotation on these resources, and tell Argo to ignore them? Just seems a bit intrusive from Skiperators point of view to have to set this annotation.

@omaen Not sure exactly what you mean, the annotation that is set here makes Argo ignore these resources. Otherwise Argo will try to delete them when terminating a namespace which causes a deadlock when skiperator's finalizers try to delete them again. It's pretty much the same as setting the prometheus/scrape annotation on behalf of the teams as we already do.

It would probably be even more intrusive if we were to do it the other way around and add some feature that made it possible for skiperator to create, but not delete resources so that argo would do deletions. Right now that is done implicitly through the ownership pattern, which tells skiperator what it owns so that it automatically cleans it up afterwards.

Probably just a poor explanation from me 😅 Just wanted to check if one could use an annotation such as skiperator.kartverket.no/managed: true on the resources that should be owned by skiperator, and in some way tell Argo to skip them. But it's not that important, either way works fine :)

omaen

Can discuss my comment further if necessary, but this solution also works as intended.
Maybe add a note about the annotation in the contributing doc?

eliihen · 2023-02-15T09:43:04Z

Just wanted to check if one could use an annotation such as skiperator.kartverket.no/managed: true on the resources that should be owned by skiperator, and in some way tell Argo to skip them

Unfortunately not, it has to be this specific annotation.

Maybe add a note about the annotation in the contributing doc?

Good idea, I'll document this property before merging

Stop argo pruning skiperator owned resources

3d6e4a4

eliihen self-assigned this Feb 14, 2023

eliihen added 2 commits February 14, 2023 12:00

Re-use argocd constant

38a6875

Add annotations to test

2fde6e0

eliihen marked this pull request as ready for review February 14, 2023 12:34

eliihen requested a review from a team as a code owner February 14, 2023 12:34

eliihen requested review from evenh, lislei and omaen February 14, 2023 12:34

evenh approved these changes Feb 14, 2023

View reviewed changes

omaen reviewed Feb 14, 2023

View reviewed changes

eliihen requested a review from omaen February 15, 2023 07:28

omaen approved these changes Feb 15, 2023

View reviewed changes

Add doc of the sync-option annotation

202ed64

eliihen added this pull request to the merge queue Feb 15, 2023

Merged via the queue into main with commit 89acbbb Feb 15, 2023

eliihen deleted the prevent-argo-prune branch February 15, 2023 09:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stop argo pruning skiperator owned resources #160

Stop argo pruning skiperator owned resources #160

eliihen commented Feb 14, 2023 •

edited

Loading

github-actions bot commented Feb 14, 2023 •

edited

Loading

evenh left a comment

evenh Feb 14, 2023 •

edited

Loading

omaen left a comment

eliihen commented Feb 15, 2023 •

edited

Loading

omaen commented Feb 15, 2023

omaen left a comment

eliihen commented Feb 15, 2023

Stop argo pruning skiperator owned resources #160

Stop argo pruning skiperator owned resources #160

Conversation

eliihen commented Feb 14, 2023 • edited Loading

github-actions bot commented Feb 14, 2023 • edited Loading

Results for sandbox – ❗ CHANGED ❗

Terraform Format and Style 🖌 success

Terraform Initialization ⚙️ success

Terraform Validation 🤖 success

Terraform Plan 📖 success

Next action 🚀

Results for dev – ❗ CHANGED ❗

Terraform Format and Style 🖌 success

Terraform Initialization ⚙️ success

Terraform Validation 🤖 success

Terraform Plan 📖 success

Next action 🚀

Results for test

Terraform Format and Style 🖌 success

Terraform Initialization ⚙️ ``

Terraform Validation 🤖 success

Terraform Plan 📖 ``

Next action 🚀

Results for prod – ❗ CHANGED ❗

Terraform Format and Style 🖌 success

Terraform Initialization ⚙️ success

Terraform Validation 🤖 success

Terraform Plan 📖 success

Next action 🚀

evenh left a comment

Choose a reason for hiding this comment

evenh Feb 14, 2023 • edited Loading

Choose a reason for hiding this comment

omaen left a comment

Choose a reason for hiding this comment

eliihen commented Feb 15, 2023 • edited Loading

omaen commented Feb 15, 2023

omaen left a comment

Choose a reason for hiding this comment

eliihen commented Feb 15, 2023

eliihen commented Feb 14, 2023 •

edited

Loading

github-actions bot commented Feb 14, 2023 •

edited

Loading

Results for sandbox – ❗ `CHANGED` ❗

Terraform Format and Style 🖌 `success`

Terraform Initialization ⚙️ `success`

Terraform Validation 🤖 `success`

Terraform Plan 📖 `success`

Results for dev – ❗ `CHANGED` ❗

Terraform Format and Style 🖌 `success`

Terraform Initialization ⚙️ `success`

Terraform Validation 🤖 `success`

Terraform Plan 📖 `success`

Terraform Format and Style 🖌 `success`

Terraform Validation 🤖 `success`

Results for prod – ❗ `CHANGED` ❗

Terraform Format and Style 🖌 `success`

Terraform Initialization ⚙️ `success`

Terraform Validation 🤖 `success`

Terraform Plan 📖 `success`

evenh Feb 14, 2023 •

edited

Loading

eliihen commented Feb 15, 2023 •

edited

Loading