fix: apply ssa namespace twice #537
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
blakepettersson
commented
Aug 17, 2023
pkg/sync/sync_context.go
Outdated
| nsSpec := &v1.Namespace{TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: kube.NamespaceKind}, ObjectMeta: metav1.ObjectMeta{Name: t.liveObj.GetName(), Annotations: t.liveObj.GetAnnotations(), Labels: t.liveObj.GetLabels(), Namespace: t.liveObj.GetNamespace()}} | ||
| liveCopy, err := kube.ToUnstructured(nsSpec) | ||
|
|
||
| message, err = sc.resourceOps.ApplyResource(context.TODO(), liveCopy, dryRunStrategy, force, validate, serverSideApply, "argocd-controller-tmp") |
There was a problem hiding this comment.
argocd-controller-tmp is a placeholder name, but the idea is that in order to preserve existing metadata we need to apply the namespace with a separate manager before using managedNamespaceMetadata on said namespace.
|
Kudos, SonarCloud Quality Gate passed!
|
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #537 +/- ##
=======================================
Coverage 54.63% 54.64%
=======================================
Files 41 41
Lines 4638 4652 +14
=======================================
+ Hits 2534 2542 +8
- Misses 1908 1912 +4
- Partials 196 198 +2
☔ View full report in Codecov by Sentry. |
blakepettersson
added a commit
to blakepettersson/argo-cd
that referenced
this pull request
Aug 17, 2023
This requires argoproj/gitops-engine#537. This is more or less similar to argoproj#13965, with the difference that this is _only_ upgrading dependencies and does not require any changes in the usage of `managedNamespaceMetadata`. Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
13 tasks
blakepettersson
added a commit
to blakepettersson/argo-cd
that referenced
this pull request
Aug 17, 2023
This requires argoproj/gitops-engine#537. This is more or less similar to argoproj#13965, with the difference that this is _only_ upgrading dependencies and does not require any changes in the usage of `managedNamespaceMetadata`. Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
blakepettersson
added a commit
to blakepettersson/argo-cd
that referenced
this pull request
Aug 18, 2023
This requires argoproj/gitops-engine#537. This builds upon argoproj#13965, with the difference that this does not require any changes in the usage of `managedNamespaceMetadata`. Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
blakepettersson
added a commit
to blakepettersson/argo-cd
that referenced
this pull request
Aug 18, 2023
This requires argoproj/gitops-engine#537. This builds upon argoproj#13965, with the difference that this does not require any changes in the usage of `managedNamespaceMetadata`. Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
blakepettersson
added a commit
to blakepettersson/argo-cd
that referenced
this pull request
Aug 18, 2023
This requires argoproj/gitops-engine#537. This builds upon argoproj#13965, with the difference that this does not require any changes in the usage of `managedNamespaceMetadata`. Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
blakepettersson
added a commit
to blakepettersson/argo-cd
that referenced
this pull request
Aug 19, 2023
This requires argoproj/gitops-engine#537. This builds upon argoproj#13965, with the difference that this does not require any changes in the usage of `managedNamespaceMetadata`. Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
blakepettersson
added a commit
to blakepettersson/argo-cd
that referenced
this pull request
Aug 19, 2023
This requires argoproj/gitops-engine#537. This builds upon argoproj#13965, with the difference that this does not require any changes in the usage of `managedNamespaceMetadata`. Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
blakepettersson
added a commit
to blakepettersson/argo-cd
that referenced
this pull request
Aug 19, 2023
Use argoproj/gitops-engine#537 version of gitops-engine, which preserves the intended behaviour; which is that an existing namespace can be "adopted" when using `managedNamespaceMetadata` without removing existing labels and annotations. Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
13 tasks
crenshaw-dev
requested changes
Sep 19, 2023
blakepettersson
added a commit
to blakepettersson/argo-cd
that referenced
this pull request
Sep 19, 2023
This is based on argoproj#13965, which this time hopefully has been updated and merged correctly to a currentish version of argo-cd. The main difference here is that the gitops version is based upon argoproj/gitops-engine#537, which should maintain pre-existing labels and annotations. Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
13 tasks
blakepettersson
force-pushed
the
fix/namespace-metadata
branch
from
c3be8a4
to
76e7caf
Compare
In argoproj/argo-cd#13965, upgrading to client-go 0.26 uncovered an issue with existing namespaces not preserving labels and annotations when converting an existing namespace to use `managedNamespaceMetadata`. From 0.26 up, in order to preserve the existing behaviour what we need to do is to first apply the existing namespace with SSA and its preexisting labels and annotations. This also needs to be applied with another manager than the default one. After that initial apply, we proceed as normal with a ServerSideApply. This is a one-time thing for any existing namespace; we check if we need to perform this action by detecting if there is a preexisting SSA annotation on the namespace. Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
blakepettersson
force-pushed
the
fix/namespace-metadata
branch
from
76e7caf
to
45d4394
Compare
Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
|
Kudos, SonarCloud Quality Gate passed!
|
14 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In argoproj/argo-cd#13965, upgrading to client-go 0.26 uncovered an issue with existing namespaces not preserving labels and annotations when converting an existing namespace to use
managedNamespaceMetadata.From 0.26 up, in order to preserve the existing behaviour what we need to do is to first apply the existing namespace with SSA and its preexisting labels and annotations. This also needs to be applied with another manager than the default one.
After that initial apply, we proceed as normal with a ServerSideApply. This is a one-time thing for any existing namespace; we check if we need to perform this action by detecting if there is a preexisting SSA annotation on the namespace.