Skip to content

Commit

Permalink
feat: enable metadata to be set on namespaces (argoproj#10672)
Browse files Browse the repository at this point in the history
* namespace labels

Signed-off-by: pashavictorovich <pavel@codefresh.io>

* create namespace should support annotations

Signed-off-by: pashavictorovich <pavel@codefresh.io>

* handle also modification hook

Signed-off-by: pashavictorovich <pavel@codefresh.io>

* regenerate entity on modify hook

Signed-off-by: pashavictorovich <pavel@codefresh.io>

* manifests

Signed-off-by: pashavictorovich <pavel@codefresh.io>

* feat: enable metadata to be set on namespaces

This builds upon the work that @pasha-codefresh did in argoproj#10288.

The main differences between this PR and the previous one is that we use
SSA to diff between different versions of the namespace, as well as
having a slightly different API in gitops-engine for setting the
namespace modifier.

We now also set the ownership of the namespace in ArgoCD.

Closes argoproj#4628
Closes argoproj#6215
Closes argoproj#7799

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* fix: don't always track namespaces

For now, only allow namespaces managed with `managedNamespaceMetadata`
to have tracking set by Argo. Ideally we'd like new namespaces to also
be tracked by Argo, but there's currently an issue with a failing
integration test.

Also wrap error message if setting the app instance errors on the
namespace.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* fix: always return true with `hasManagedMetadata`

If `hasManagedMetadata` is set, `true` should always be returned.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* docs: add clarifying docs on resource tracking

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* style: pr tweaks

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* fix: re-add label unsetting

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* Update gitops-engine to current master

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

Signed-off-by: pashavictorovich <pavel@codefresh.io>
Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Co-authored-by: pashavictorovich <pavel@codefresh.io>
Co-authored-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Signed-off-by: emirot <emirot.nolan@gmail.com>
  • Loading branch information
3 people authored and emirot committed Jan 27, 2023
1 parent ed7f7b6 commit bafebe9
Show file tree
Hide file tree
Showing 25 changed files with 3,198 additions and 735 deletions.
20 changes: 20 additions & 0 deletions assets/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -6544,6 +6544,23 @@
}
}
},
"v1alpha1ManagedNamespaceMetadata": {
"type": "object",
"properties": {
"annotations": {
"type": "object",
"additionalProperties": {
"type": "string"
}
},
"labels": {
"type": "object",
"additionalProperties": {
"type": "string"
}
}
}
},
"v1alpha1MatrixGenerator": {
"description": "MatrixGenerator generates the cartesian product of two sets of parameters. The parameters are defined by two nested\ngenerators.",
"type": "object",
Expand Down Expand Up @@ -7827,6 +7844,9 @@
"automated": {
"$ref": "#/definitions/v1alpha1SyncPolicyAutomated"
},
"managedNamespaceMetadata": {
"$ref": "#/definitions/v1alpha1ManagedNamespaceMetadata"
},
"retry": {
"$ref": "#/definitions/v1alpha1RetryStrategy"
},
Expand Down
33 changes: 17 additions & 16 deletions controller/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"fmt"
cdcommon "github.com/argoproj/argo-cd/v2/common"
"os"
"strconv"
"sync/atomic"
Expand All @@ -20,7 +21,6 @@ import (
"k8s.io/apimachinery/pkg/util/managedfields"
"k8s.io/kubectl/pkg/util/openapi"

cdcommon "github.com/argoproj/argo-cd/v2/common"
"github.com/argoproj/argo-cd/v2/controller/metrics"
"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
listersv1alpha1 "github.com/argoproj/argo-cd/v2/pkg/client/listers/application/v1alpha1"
Expand Down Expand Up @@ -212,14 +212,7 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha
}
trackingMethod := argo.GetTrackingMethod(m.settingsMgr)

syncCtx, cleanup, err := sync.NewSyncContext(
compareResult.syncStatus.Revision,
reconciliationResult,
restConfig,
rawConfig,
m.kubectl,
app.Spec.Destination.Namespace,
openAPISchema,
opts := []sync.SyncOpt{
sync.WithLogr(logutils.NewLogrusLogger(logEntry)),
sync.WithHealthOverride(lua.ResourceHealthOverrides(resourceOverrides)),
sync.WithPermissionValidator(func(un *unstructured.Unstructured, res *v1.APIResource) error {
Expand Down Expand Up @@ -249,20 +242,28 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha
m.isSelfReferencedObj(live, target, app.GetName(), appLabelKey, trackingMethod)
}),
sync.WithManifestValidation(!syncOp.SyncOptions.HasOption(common.SyncOptionsDisableValidation)),
sync.WithNamespaceCreation(syncOp.SyncOptions.HasOption("CreateNamespace=true"), func(un *unstructured.Unstructured) bool {
if un != nil && kube.GetAppInstanceLabel(un, cdcommon.LabelKeyAppInstance) != "" {
kube.UnsetLabel(un, cdcommon.LabelKeyAppInstance)
return true
}
return false
}),
sync.WithSyncWaveHook(delayBetweenSyncWaves),
sync.WithPruneLast(syncOp.SyncOptions.HasOption(common.SyncOptionPruneLast)),
sync.WithResourceModificationChecker(syncOp.SyncOptions.HasOption("ApplyOutOfSyncOnly=true"), compareResult.diffResultList),
sync.WithPrunePropagationPolicy(&prunePropagationPolicy),
sync.WithReplace(syncOp.SyncOptions.HasOption(common.SyncOptionReplace)),
sync.WithServerSideApply(syncOp.SyncOptions.HasOption(common.SyncOptionServerSideApply)),
sync.WithServerSideApplyManager(cdcommon.ArgoCDSSAManager),
}

if syncOp.SyncOptions.HasOption("CreateNamespace=true") {
opts = append(opts, sync.WithNamespaceModifier(syncNamespace(m.resourceTracking, appLabelKey, trackingMethod, app.Name, app.Spec.SyncPolicy)))
}

syncCtx, cleanup, err := sync.NewSyncContext(
compareResult.syncStatus.Revision,
reconciliationResult,
restConfig,
rawConfig,
m.kubectl,
app.Spec.Destination.Namespace,
openAPISchema,
opts...,
)

if err != nil {
Expand Down
51 changes: 51 additions & 0 deletions controller/sync_namespace.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package controller

import (
"fmt"
cdcommon "github.com/argoproj/argo-cd/v2/common"
"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
"github.com/argoproj/argo-cd/v2/util/argo"
gitopscommon "github.com/argoproj/gitops-engine/pkg/sync/common"
"github.com/argoproj/gitops-engine/pkg/utils/kube"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)

func syncNamespace(resourceTracking argo.ResourceTracking, appLabelKey string, trackingMethod v1alpha1.TrackingMethod, appName string, syncPolicy *v1alpha1.SyncPolicy) func(un *unstructured.Unstructured) (bool, error) {
return func(liveNs *unstructured.Unstructured) (bool, error) {
if liveNs != nil && kube.GetAppInstanceLabel(liveNs, cdcommon.LabelKeyAppInstance) != "" {
kube.UnsetLabel(liveNs, cdcommon.LabelKeyAppInstance)
return true, nil
}

isNewNamespace := liveNs != nil && liveNs.GetUID() == "" && liveNs.GetResourceVersion() == ""

if liveNs != nil && syncPolicy != nil {
// managedNamespaceMetadata relies on SSA, and since the diffs are computed by the k8s control plane we
// always need to call the k8s api server, so we'll always need to return true if managedNamespaceMetadata is set.
hasManagedMetadata := syncPolicy.ManagedNamespaceMetadata != nil
if hasManagedMetadata {
managedNamespaceMetadata := syncPolicy.ManagedNamespaceMetadata
liveNs.SetLabels(managedNamespaceMetadata.Labels)
liveNs.SetAnnotations(appendSSAAnnotation(managedNamespaceMetadata.Annotations))

err := resourceTracking.SetAppInstance(liveNs, appLabelKey, appName, "", trackingMethod)
if err != nil {
return false, fmt.Errorf("failed to set app instance tracking on the namespace %s: %s", liveNs.GetName(), err)
}

return true, nil
}
}

return isNewNamespace, nil
}
}

func appendSSAAnnotation(in map[string]string) map[string]string {
r := map[string]string{}
for k, v := range in {
r[k] = v
}
r[gitopscommon.AnnotationSyncOptions] = gitopscommon.SyncOptionServerSideApply
return r
}
Loading

0 comments on commit bafebe9

Please sign in to comment.