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

test: deduplicate test manifests and fix "stress" profile #14001

Merged
merged 5 commits into from
Dec 31, 2024

Conversation

MasonM
Copy link
Contributor

@MasonM MasonM commented Dec 16, 2024

Motivation

The duplication of the Kustomization files under test/e2e/manifests/ makes them hard to follow. Also, the "stress" profile is broken, which I'm guessing was introduced in 87cb155:

$ make install PROFILE=stress
...
error: accumulating resources: accumulation err='merging resources from 'workflow-controller-podpriorityclass.yaml': may not add resource with an already registered id: PriorityClass.v1.scheduling.k8s.io/workflow-controller.[noNs]': must build at directory: '/home/vscode/go/src/github.com/argoproj/argo-workflows/test/e2e/manifests/stress/workflow-controller-podpriorityclass.yaml': file is not directory

Modifications

I used Kustomize Components to clean up the manifests by extracting common configuration to a base component.

I also added a local-argo component to handle scaling workflow-controller and argo-server so we can remove the corresponding lines from Makefile, since it's cleaner to let kustomize handle that.

Verification

I used the following script to verify nothing unexpected changed:

set -e
for PROFILE in events minimal mysql plugins postgres prometheus sso; do
  echo "diffing $PROFILE"
  git checkout --quiet main
  kubectl kustomize --load-restrictor=LoadRestrictionsNone test/e2e/manifests/$PROFILE > "old_$PROFILE.yaml"
  git checkout --quiet deduplicate-manifest
  kubectl kustomize --load-restrictor=LoadRestrictionsNone test/e2e/manifests/$PROFILE > "new_$PROFILE.yaml"
  diff -U3 "old_$PROFILE.yaml" "new_$PROFILE.yaml" || true
done

Output:

diffing events
--- old_events.yaml	2024-12-16 04:33:35.012533594 +0000
+++ new_events.yaml	2024-12-16 04:33:35.242545139 +0000
@@ -3448,11 +3448,12 @@
   - clusterworkflowtemplates
   - clusterworkflowtemplates/finalizers
   verbs:
+  - create
+  - delete
+  - watch
   - get
   - list
   - watch
-  - create
-  - delete
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: RoleBinding
@@ -3759,6 +3760,17 @@
 ---
 apiVersion: v1
 kind: Secret
+metadata:
+  annotations:
+    kubernetes.io/service-account.name: argo-server
+  labels:
+    app.kubernetes.io/part-of: argo
+  name: argo-server.service-account-token
+  namespace: argo
+type: kubernetes.io/service-account-token
+---
+apiVersion: v1
+kind: Secret
 metadata:
   labels:
     app.kubernetes.io/part-of: argo
diffing minimal
--- old_minimal.yaml	2024-12-16 04:33:35.498557988 +0000
+++ new_minimal.yaml	2024-12-16 04:33:35.776571942 +0000
@@ -3341,11 +3341,12 @@
   - clusterworkflowtemplates
   - clusterworkflowtemplates/finalizers
   verbs:
+  - create
+  - delete
+  - watch
   - get
   - list
   - watch
-  - create
-  - delete
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRole
diffing mysql
--- old_mysql.yaml	2024-12-16 04:33:36.013583837 +0000
+++ new_mysql.yaml	2024-12-16 04:33:36.279597189 +0000
@@ -3333,11 +3333,12 @@
   - clusterworkflowtemplates
   - clusterworkflowtemplates/finalizers
   verbs:
+  - create
+  - delete
+  - watch
   - get
   - list
   - watch
-  - create
-  - delete
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: RoleBinding
diffing plugins
--- old_plugins.yaml	2024-12-16 04:33:36.553610941 +0000
+++ new_plugins.yaml	2024-12-16 04:33:36.834625045 +0000
@@ -3349,11 +3349,12 @@
   - clusterworkflowtemplates
   - clusterworkflowtemplates/finalizers
   verbs:
+  - create
+  - delete
+  - watch
   - get
   - list
   - watch
-  - create
-  - delete
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRole
diffing postgres
--- old_postgres.yaml	2024-12-16 04:33:37.103638547 +0000
+++ new_postgres.yaml	2024-12-16 04:33:37.378652351 +0000
@@ -3333,11 +3333,12 @@
   - clusterworkflowtemplates
   - clusterworkflowtemplates/finalizers
   verbs:
+  - create
+  - delete
+  - watch
   - get
   - list
   - watch
-  - create
-  - delete
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: RoleBinding
diffing prometheus
--- old_prometheus.yaml	2024-12-16 04:33:37.560661486 +0000
+++ new_prometheus.yaml	2024-12-16 04:33:37.747670872 +0000
@@ -3200,11 +3200,12 @@
   - clusterworkflowtemplates
   - clusterworkflowtemplates/finalizers
   verbs:
+  - create
+  - delete
+  - watch
   - get
   - list
   - watch
-  - create
-  - delete
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: RoleBinding
@@ -3499,6 +3500,16 @@
 apiVersion: v1
 kind: Secret
 metadata:
+  annotations:
+    kubernetes.io/service-account.name: argo-server
+  labels:
+    app.kubernetes.io/part-of: argo
+  name: argo-server.service-account-token
+type: kubernetes.io/service-account-token
+---
+apiVersion: v1
+kind: Secret
+metadata:
   labels:
     app.kubernetes.io/part-of: argo
   name: argo-workflows-webhook-clients
@@ -3728,7 +3739,6 @@
   name: argo-server
   namespace: argo
 spec:
-  replicas: 0
   selector:
     matchLabels:
       app: argo-server
@@ -3785,7 +3795,6 @@
   name: workflow-controller
   namespace: argo
 spec:
-  replicas: 0
   selector:
     matchLabels:
       app: workflow-controller
diffing sso
--- old_sso.yaml	2024-12-16 04:33:37.920679555 +0000
+++ new_sso.yaml	2024-12-16 04:33:38.179692555 +0000
@@ -3236,11 +3236,12 @@
   - clusterworkflowtemplates
   - clusterworkflowtemplates/finalizers
   verbs:
+  - create
+  - delete
+  - watch
   - get
   - list
   - watch
-  - create
-  - delete
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: RoleBinding

This deduplicates the test manifests using [Kustomize
Components](https://kubectl.docs.kubernetes.io/guides/config_management/components/)
and fixes the "stress" profile, which it seems has been broken ever
since `87cb1559107ec88dd418229b38113d70ba2a8580`. Running `make install
PROFILE=stress` on the `main` branch gives this error:
```
error: accumulating resources: accumulation err='merging resources from 'workflow-controller-podpriorityclass.yaml': may not add resource with an already registered id: PriorityClass.v1.scheduling.k8s.io/workflow-controller.[noNs]': must build at directory: '/home/vscode/go/src/github.com/argoproj/argo-workflows/test/e2e/manifests/stress/workflow-controller-podpriorityclass.yaml': file is not directory
```

I also used a component to handle scaling
`deploy/workflow-controller`/`deploy/argo-server` so we can remove those
lines from `Makefile`.

I used the following script to verify nothing unexpected changed:
```
set -e
for PROFILE in events minimal mysql plugins postgres prometheus sso; do
  echo "diffing $PROFILE"
  git checkout --quiet main
  kubectl kustomize --load-restrictor=LoadRestrictionsNone test/e2e/manifests/$PROFILE > "old_$PROFILE.yaml"
  git checkout --quiet deduplicate-manifest
  kubectl kustomize --load-restrictor=LoadRestrictionsNone test/e2e/manifests/$PROFILE > "new_$PROFILE.yaml"
  diff -U3 "old_$PROFILE.yaml" "new_$PROFILE.yaml" || true
done
```

Output:
```
diffing events
--- old_events.yaml	2024-12-16 03:45:49.404387813 +0000
+++ new_events.yaml	2024-12-16 03:45:49.650400191 +0000
@@ -3760,6 +3760,17 @@
 apiVersion: v1
 kind: Secret
 metadata:
+  annotations:
+    kubernetes.io/service-account.name: argo-server
+  labels:
+    app.kubernetes.io/part-of: argo
+  name: argo-server.service-account-token
+  namespace: argo
+type: kubernetes.io/service-account-token
+---
+apiVersion: v1
+kind: Secret
+metadata:
   labels:
     app.kubernetes.io/part-of: argo
   name: argo-workflows-webhook-clients
diffing minimal
diffing mysql
diffing plugins
diffing postgres
diffing prometheus
--- old_prometheus.yaml	2024-12-16 03:45:52.009518890 +0000
+++ new_prometheus.yaml	2024-12-16 03:45:52.203528651 +0000
@@ -3499,6 +3499,16 @@
 apiVersion: v1
 kind: Secret
 metadata:
+  annotations:
+    kubernetes.io/service-account.name: argo-server
+  labels:
+    app.kubernetes.io/part-of: argo
+  name: argo-server.service-account-token
+type: kubernetes.io/service-account-token
+---
+apiVersion: v1
+kind: Secret
+metadata:
   labels:
     app.kubernetes.io/part-of: argo
   name: argo-workflows-webhook-clients
@@ -3728,7 +3738,6 @@
   name: argo-server
   namespace: argo
 spec:
-  replicas: 0
   selector:
     matchLabels:
       app: argo-server
@@ -3785,7 +3794,6 @@
   name: workflow-controller
   namespace: argo
 spec:
-  replicas: 0
   selector:
     matchLabels:
       app: workflow-controller
diffing sso
```

Signed-off-by: Mason Malone <[email protected]>
Signed-off-by: Mason Malone <[email protected]>
@MasonM MasonM marked this pull request as ready for review December 16, 2024 05:08
@MasonM
Copy link
Contributor Author

MasonM commented Dec 22, 2024

/retest

@tczhao tczhao enabled auto-merge (squash) December 30, 2024 13:41
@tczhao
Copy link
Member

tczhao commented Dec 30, 2024

@MasonM Could you fix the conflicts please

@tczhao tczhao merged commit 81993b5 into argoproj:main Dec 31, 2024
31 checks passed
MasonM added a commit to MasonM/argo-workflows that referenced this pull request Jan 3, 2025
This was split off from
argoproj#5114 to make it easier
to review. This makes a few minor improvements to the build to support
the full CRD fixes:
1. Update devcontainer to install k3s 1.29.10, since that's what we use
   in CI: https://github.com/argoproj/argo-workflows/blob/ef41f83f801a6ff48c87f882a6b75d0e37529134/.github/workflows/ci-build.yaml#L263
   1.27 is EOL and doesn't support things like validation rules in CRDs.
2. Update `make install` to use [server-side
   apply](https://kubernetes.io/docs/reference/using-api/server-side-apply/),
   which is meant to replace client-side apply and isn't affected by
   size limitations for the CRDs. Since `kubectl apply --server-side`
   isn't compatible with `kubectl apply --prune`, I had to switch to
   [apply
   sets](https://kubernetes.io/docs/tasks/manage-kubernetes-objects/declarative-config/#alternative-kubectl-apply-f-directory-prune),
   which is intended to replace allow list pruning, and seems to work
   just as wlel.
3. Minor refactoring of the manifests under `manifests/` to use
   [Kustomize
   Components](https://kubectl.docs.kubernetes.io/guides/config_management/components/)
   so that we can share code with the the manifests under
   `test/e2e/manifests` without duplication. See argoproj#14001 for more details
   on this approach.

Signed-off-by: Mason Malone <[email protected]>
MasonM added a commit to MasonM/argo-workflows that referenced this pull request Jan 3, 2025
This was split off from
argoproj#14044 to make it easier
to review. This makes a few minor improvements to the build to support
the full CRD fixes:
1. Update devcontainer to install k3s 1.29.10, since that's what we use
   in CI: https://github.com/argoproj/argo-workflows/blob/ef41f83f801a6ff48c87f882a6b75d0e37529134/.github/workflows/ci-build.yaml#L263
   1.27 is EOL and doesn't support things like validation rules in CRDs.
2. Update `make install` to use [server-side
   apply](https://kubernetes.io/docs/reference/using-api/server-side-apply/),
   which is meant to replace client-side apply and isn't affected by
   size limitations for the CRDs. Since `kubectl apply --server-side`
   isn't compatible with `kubectl apply --prune`, I had to switch to
   [apply
   sets](https://kubernetes.io/docs/tasks/manage-kubernetes-objects/declarative-config/#alternative-kubectl-apply-f-directory-prune),
   which is intended to replace allow list pruning, and seems to work
   just as wlel.
3. Minor refactoring of the manifests under `manifests/` to use
   [Kustomize
   Components](https://kubectl.docs.kubernetes.io/guides/config_management/components/)
   so that we can share code with the the manifests under
   `test/e2e/manifests` without duplication. See argoproj#14001 for more details
   on this approach.

Signed-off-by: Mason Malone <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants