ServerSideApply breaks CreateNamespace #13874

morey-tech · 2023-06-01T15:30:21Z

Checklist:

I've searched in the docs and FAQ for my answer: https://bit.ly/argocd-faq.
I've included steps to reproduce the bug.
I've pasted the output of argocd version.

Describe the bug

When ServerSideApply is enabled on a resource (e.g., a ConfigMap or Deployment) and the Application is using CreateNamespace, the sync will fail with namespaces "<namespace>" not found.

Based on this thread in the #argo-cd channel.

To Reproduce

Create new application using this spec:

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: 'kustomize-guestbook'
spec:
  project: default
  source:
    repoURL: 'https://github.com/morey-tech/argocd-example-apps/'
    path: general/kustomize-guestbook
    targetRevision: ssa-createnamespace-test
    kustomize:
      namespace: kustomize-guestbook
  destination:
    namespace: kustomize-guestbook
    name: kind
  syncPolicy:
    automated:
      selfHeal: true
    syncOptions:
      - CreateNamespace=true

Expected behavior

Without ServerSideApply on a resource, the sync creates the namespace and the resources without issue. I would expect the same here. Here's a working example (the only difference is no SSA on the deployment):

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: 'kustomize-guestbook'
spec:
  project: default
  source:
    repoURL: 'https://github.com/morey-tech/argocd-example-apps/'
    path: general/kustomize-guestbook
    targetRevision: HEAD
  destination:
    namespace: kustomize-guestbook
    name: kind
  syncPolicy:
    automated:
      selfHeal: true
    syncOptions:
      - CreateNamespace=true

Version

{
    "Version": "v2.7.2+cbee7e6.dirty",
    "BuildDate": "2023-05-12T13:43:26Z",
    "GitCommit": "cbee7e6011407ed2d1066c482db74e97e0cc6bdb",
    "GitTreeState": "dirty",
    "GoVersion": "go1.19.6",
    "Compiler": "gc",
    "Platform": "linux/amd64",
    "KustomizeVersion": "v5.0.1 2023-03-14T01:32:48Z",
    "HelmVersion": "v3.11.2+g912ebc1",
    "KubectlVersion": "v0.24.2",
    "JsonnetVersion": "v0.19.1"
}

Logs

Paste any relevant application logs here.

The text was updated successfully, but these errors were encountered:

morey-tech · 2023-06-01T15:36:23Z

@leoluz, is this expected behaviour with the implementation of #9711?

blakepettersson · 2023-06-02T07:18:32Z

Does this work when adding managedNamespaceMetadata, like

  syncPolicy:
    automated:
      selfHeal: true
    managedNamespaceMetadata:
      labels: {}
      annotations: {}
    syncOptions:
      - CreateNamespace=true

nicl-dev · 2023-06-02T07:51:38Z

We tried that already. Unfortunately it doesn't.

morey-tech · 2023-06-02T13:36:54Z

In my example, the addition of spec.syncPolicy.managedNamespaceMetadata={} does solve the problem.

  apiVersion: argoproj.io/v1alpha1
  kind: Application
  metadata:
    name: 'kustomize-guestbook'
  spec:
    project: default
    source:
      repoURL: 'https://github.com/morey-tech/argocd-example-apps/'
      path: general/kustomize-guestbook
      targetRevision: ssa-createnamespace-test
    destination:
      namespace: kustomize-guestbook
      name: kind
    syncPolicy:
      automated:
        selfHeal: true
      syncOptions:
        - CreateNamespace=true
++    managedNamespaceMetadata: {}

morey-tech · 2023-06-02T16:19:50Z

@nicl-dev, can you provide the steps to reproduce the issue with managedNamespaceMetadata in use?

nicl-dev · 2023-06-02T16:27:24Z

Yea sure, I will try to reproduce it on Monday. My colleague was 100% sure he tried that solution but I'll try it again.

leoluz · 2023-06-02T17:15:11Z

@leoluz, is this expected behaviour with the implementation of #9711?

@morey-tech Not an expected behaviour. If it doesn't, it might be a bug.

In my example, the addition of spec.syncPolicy.managedNamespaceMetadata={} does solve the problem.

@morey-tech That is surprising because enabling managedNamespaceMetadata will create the namespace with server-side apply.

morey-tech · 2023-06-02T17:37:00Z

In my example, the addition of spec.syncPolicy.managedNamespaceMetadata={} does solve the problem.

@morey-tech That is surprising because enabling managedNamespaceMetadata will create the namespace with server-side apply.

@leoluz, this almost makes sense then right? Without that, the Deployment uses SSA, but the namespace isn't. So SSA complains that the namespace doesn't exist. But with managedNamespaceMetadata, both the deployment and namespace are included in the SSA, and therefore the cluster knows the namespace will exist before applying the deployment.

nicl-dev · 2023-06-05T08:50:25Z

Alright, I just tested it again and managedNamespaceMetadata does indeed fix the problem. Sorry for the confusion.

jaideepr97 · 2023-06-05T13:58:06Z

closing this issue since it sounds like we have reached a resolution
please feel free to re-open if that is not the case

morey-tech · 2023-06-05T14:13:54Z

@jaideepr97 the resolution reached was merely a way to mitigate this issue's bug. I assume it's not the desired behaviour for any resource using ServerSideApply to cause CreateNamespace to malfunction when managedNamespaceMetadata is absent.

It seems like automatically including managedNamespaceMetadata: {} when CreateNamespace is in use, along with ServerSideApply, would resolve this bug. But maybe there is a more intelligent solution.

evs-ops · 2023-11-23T13:15:03Z

I can confirm that also in version v2.9.2 the problem exists. I tried to use managedNamespaceMetadata on an applicationset with ServerSideApply=true and it didn't do anything. Once I removed ServerSideApply it worked like a charm

blakepettersson · 2024-01-12T19:38:35Z

@evs-ops can you show the Application[Set] that displays this issue?

It seems like automatically including managedNamespaceMetadata: {} when CreateNamespace is in use, along with ServerSideApply, would resolve this bug. But maybe there is a more intelligent solution.

The issue is that gitops-engine will already SSA the namespace if the [whole] application itself has ServerSideApply as a sync option. In this example it's only the Deployment that has SSA applied to it and since the Application doesn't have SSA on it, the namespace in turn will not be SSA'd. Not sure what the right course of action should be here.

anandf · 2024-01-16T08:08:24Z

The fix done for #16829 via PR argoproj/gitops-engine#563 seems to fix this issue as well.

I used the following application to reproduce the error (in v2.10.0-rc1) and validate the fix with the modified code.

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: kustomize-guestbook
  namespace: argocd
spec:
  destination:
    namespace: kustomize-guestbook
    server: https://kubernetes.default.svc
  project: default
  source:
    kustomize:
      namespace: kustomize-guestbook
    path: general/kustomize-guestbook
    repoURL: https://github.com/morey-tech/argocd-example-apps/
    targetRevision: ssa-createnamespace-test
  syncPolicy:
    automated:
      selfHeal: true
    syncOptions:
      - CreateNamespace=true

leoluz · 2024-01-17T16:48:48Z

I ran some tests yesterday and I understand the problem:

Server side apply was implemented configuring the kubectl library to set the ServerSideApply flag with what was defined in the sync context.
During sync Argo CD will first (by default) apply all resources in dry-run mode to make sure that all yamls are valid.
If sync context is set to run with ServerSideApply, it will also run the dry-run phase in that mode.
By the time the server-side apply was implemented, the dry-run phase was always executed with dry-run=client (which was ignored by the kubectl lib here).
This caused this bug and it was attempted to be fixed by setting the dry-run=server during ServerSideApply syncs in this PR.
Setting dry-run=server creates a new problem as it will validate if the namespace exists in the server during the dry-run phase. This fails syncs for apps that request to auto create namespace as described in this issue.

In my opinion the better way to solve this problem is, regardless if the sync context is defined to sync with ServerSideApply, the dry-run phase should always run with client-side-apply and dry-run=client. This will address all the problems mentioned above and the implementation will be simpler. I think that this direction should be taken instead of this, this and this.

Related to #16829

cc @pasha-codefresh , @anandf, @morey-tech

MattiasAng · 2024-01-17T21:32:05Z

@leoluz I am not gonna say what's wrong or right in terms of implementation, but I think it is important to know that a server dry run will catch issues that would not be shown in a client dry run. For us, a client dry run is basically useless for what we're trying to test before applying a manifest.

A good example of this is when using any form of policy controller that validates the manifest, for example Kyverno.

While I understand that the first reasoning behind implementing it might have been the wrong direction, I think having the ability to run with dry-run=server would be a nice-to-have feature.

leoluz · 2024-01-17T21:54:42Z

While I understand that the first reasoning behind implementing it might have been the wrong direction, I think having the ability to run with dry-run=server would be a nice-to-have feature.

@MattiasAng I agree with you and the feature that you described is already implemented and will be available as part of Argo CD 2.10 release. It is called ServerSide Diff and when enabled will calculate diffs using server-side apply in dryrun mode. This will take into consideration validation and mutation webhooks use-cases like what you are describing. We blogged about it here.

The current dry-run phase that is currently executed on every sync will be obsolete (and possibly removed) when server-side diff is promoted as the main diff strategy.

MattiasAng · 2024-01-18T07:52:48Z

@leoluz Thanks for the clarification, seems like I was missing some context.

leoluz · 2024-01-23T20:44:08Z

@morey-tech Given that we just merged the PRs(argoproj/gitops-engine#564 and #16942) with the fix suggested here can you give it another try with the latest master build?

leoluz · 2024-02-06T20:48:02Z

Closing this for now as this is addressed by argoproj/gitops-engine#564 and #16942.
@morey-tech please feel free to reopen if the issue remains.

morey-tech added the bug Something isn't working label Jun 1, 2023

jaideepr97 closed this as completed Jun 5, 2023

morey-tech reopened this Jun 5, 2023

crenshaw-dev added the server-side-apply label Oct 31, 2023

rbreeze mentioned this issue Jan 11, 2024

[2.10 RC] Namespace not created in ArgoCD v2.10.0-rc1 when using CreateNamespace syncOption #16829

Closed

leoluz mentioned this issue Jan 11, 2024

AppController: error calculating server side diff with 2.10-rc1 #16840

Closed

pasha-codefresh mentioned this issue Jan 16, 2024

fix(server): Application sync fails when CreateNamespace and ServerSideApply flags are set in the sync options argoproj/gitops-engine#563

Closed

leoluz mentioned this issue Jan 23, 2024

fix(appcontroller): Uptake fix in gitops engine which fixes application sync with auto create ns and server side apply (cherry-pick #16942) #16963

Merged

leoluz closed this as completed Feb 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ServerSideApply breaks CreateNamespace #13874

ServerSideApply breaks CreateNamespace #13874

morey-tech commented Jun 1, 2023 •

edited

Loading

morey-tech commented Jun 1, 2023

blakepettersson commented Jun 2, 2023

nicl-dev commented Jun 2, 2023

morey-tech commented Jun 2, 2023

morey-tech commented Jun 2, 2023

nicl-dev commented Jun 2, 2023

leoluz commented Jun 2, 2023

morey-tech commented Jun 2, 2023 •

edited

Loading

nicl-dev commented Jun 5, 2023

jaideepr97 commented Jun 5, 2023

morey-tech commented Jun 5, 2023

evs-ops commented Nov 23, 2023

blakepettersson commented Jan 12, 2024

anandf commented Jan 16, 2024

leoluz commented Jan 17, 2024 •

edited

Loading

MattiasAng commented Jan 17, 2024

leoluz commented Jan 17, 2024 •

edited

Loading

MattiasAng commented Jan 18, 2024

leoluz commented Jan 23, 2024 •

edited

Loading

leoluz commented Feb 6, 2024

ServerSideApply breaks CreateNamespace #13874

ServerSideApply breaks CreateNamespace #13874

Comments

morey-tech commented Jun 1, 2023 • edited Loading

morey-tech commented Jun 1, 2023

blakepettersson commented Jun 2, 2023

nicl-dev commented Jun 2, 2023

morey-tech commented Jun 2, 2023

morey-tech commented Jun 2, 2023

nicl-dev commented Jun 2, 2023

leoluz commented Jun 2, 2023

morey-tech commented Jun 2, 2023 • edited Loading

nicl-dev commented Jun 5, 2023

jaideepr97 commented Jun 5, 2023

morey-tech commented Jun 5, 2023

evs-ops commented Nov 23, 2023

blakepettersson commented Jan 12, 2024

anandf commented Jan 16, 2024

leoluz commented Jan 17, 2024 • edited Loading

MattiasAng commented Jan 17, 2024

leoluz commented Jan 17, 2024 • edited Loading

MattiasAng commented Jan 18, 2024

leoluz commented Jan 23, 2024 • edited Loading

leoluz commented Feb 6, 2024

morey-tech commented Jun 1, 2023 •

edited

Loading

morey-tech commented Jun 2, 2023 •

edited

Loading

leoluz commented Jan 17, 2024 •

edited

Loading

leoluz commented Jan 17, 2024 •

edited

Loading

leoluz commented Jan 23, 2024 •

edited

Loading