forked from argoproj/gitops-engine
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: Server side diff now works correctly with some fields removal
Helps with argoproj/argo-cd#20792 Removed and modified sets may only contain the fields that changed, not including key fields like "name". This can cause merge to fail, since it expects those fields to be present if they are present in the predicted live. Fortunately, we can inspect the set and derive the key fields necessary. Then they can be added to the set and used during a merge. Also, have a new test which fails before the fix, but passes now. Failure of the new test before the fix ``` Error: Received unexpected error: error removing non config mutations for resource Deployment/nginx-deployment: error reverting webhook removed fields in predicted live resource: .spec.template.spec.containers: element 0: associative list with keys has an element that omits key field "name" (and doesn't have default value) Test: TestServerSideDiff/will_test_removing_some_field_with_undoing_changes_done_by_webhook ``` Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>
- Loading branch information
1 parent
88c35a9
commit e881697
Showing
7 changed files
with
465 additions
and
2 deletions.
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| apiVersion: apps/v1 | ||
| kind: Deployment | ||
| metadata: | ||
| labels: | ||
| app: missing | ||
| applications.argoproj.io/app-name: nginx | ||
| something-else: bla | ||
| name: nginx-deployment | ||
| namespace: default | ||
| spec: | ||
| replicas: 2 | ||
| selector: | ||
| matchLabels: | ||
| app: nginx | ||
| template: | ||
| metadata: | ||
| labels: | ||
| app: nginx | ||
| applications.argoproj.io/app-name: nginx | ||
| spec: | ||
| containers: | ||
| - image: 'nginx:1.23.1' | ||
| imagePullPolicy: Never | ||
| livenessProbe: | ||
| exec: | ||
| command: | ||
| - cat | ||
| - non-existent-file | ||
| initialDelaySeconds: 5 | ||
| periodSeconds: 180 | ||
| name: nginx | ||
| ports: | ||
| - containerPort: 8081 | ||
| protocol: UDP | ||
| - containerPort: 80 | ||
| protocol: TCP |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,161 @@ | ||
| apiVersion: apps/v1 | ||
| kind: Deployment | ||
| metadata: | ||
| annotations: | ||
| deployment.kubernetes.io/revision: '1' | ||
| creationTimestamp: '2022-09-18T23:50:25Z' | ||
| generation: 1 | ||
| labels: | ||
| app: missing | ||
| applications.argoproj.io/app-name: nginx | ||
| something-else: bla | ||
| managedFields: | ||
| - apiVersion: apps/v1 | ||
| fieldsType: FieldsV1 | ||
| fieldsV1: | ||
| 'f:metadata': | ||
| 'f:labels': | ||
| 'f:app': {} | ||
| 'f:applications.argoproj.io/app-name': {} | ||
| 'f:something-else': {} | ||
| 'f:spec': | ||
| 'f:replicas': {} | ||
| 'f:selector': {} | ||
| 'f:template': | ||
| 'f:metadata': | ||
| 'f:labels': | ||
| 'f:app': {} | ||
| 'f:applications.argoproj.io/app-name': {} | ||
| 'f:spec': | ||
| 'f:containers': | ||
| 'k:{"name":"nginx"}': | ||
| .: {} | ||
| 'f:image': {} | ||
| 'f:imagePullPolicy': {} | ||
| 'f:livenessProbe': | ||
| 'f:exec': | ||
| 'f:command': {} | ||
| 'f:initialDelaySeconds': {} | ||
| 'f:periodSeconds': {} | ||
| 'f:name': {} | ||
| 'f:ports': | ||
| 'k:{"containerPort":80,"protocol":"TCP"}': | ||
| .: {} | ||
| 'f:containerPort': {} | ||
| 'f:protocol': {} | ||
| 'f:resources': | ||
| 'f:requests': | ||
| 'f:cpu': {} | ||
| 'f:memory': {} | ||
| manager: argocd-controller | ||
| operation: Apply | ||
| time: '2022-09-18T23:50:25Z' | ||
| - apiVersion: apps/v1 | ||
| fieldsType: FieldsV1 | ||
| fieldsV1: | ||
| 'f:metadata': | ||
| 'f:annotations': | ||
| .: {} | ||
| 'f:deployment.kubernetes.io/revision': {} | ||
| 'f:status': | ||
| 'f:availableReplicas': {} | ||
| 'f:conditions': | ||
| .: {} | ||
| 'k:{"type":"Available"}': | ||
| .: {} | ||
| 'f:lastTransitionTime': {} | ||
| 'f:lastUpdateTime': {} | ||
| 'f:message': {} | ||
| 'f:reason': {} | ||
| 'f:status': {} | ||
| 'f:type': {} | ||
| 'k:{"type":"Progressing"}': | ||
| .: {} | ||
| 'f:lastTransitionTime': {} | ||
| 'f:lastUpdateTime': {} | ||
| 'f:message': {} | ||
| 'f:reason': {} | ||
| 'f:status': {} | ||
| 'f:type': {} | ||
| 'f:observedGeneration': {} | ||
| 'f:readyReplicas': {} | ||
| 'f:replicas': {} | ||
| 'f:updatedReplicas': {} | ||
| manager: kube-controller-manager | ||
| operation: Update | ||
| subresource: status | ||
| time: '2022-09-23T18:30:59Z' | ||
| name: nginx-deployment | ||
| namespace: default | ||
| resourceVersion: '7492752' | ||
| uid: 731f7434-d3d9-47fa-b179-d9368a84f7c9 | ||
| spec: | ||
| progressDeadlineSeconds: 600 | ||
| replicas: 2 | ||
| revisionHistoryLimit: 10 | ||
| selector: | ||
| matchLabels: | ||
| app: nginx | ||
| strategy: | ||
| rollingUpdate: | ||
| maxSurge: 25% | ||
| maxUnavailable: 25% | ||
| type: RollingUpdate | ||
| template: | ||
| metadata: | ||
| creationTimestamp: null | ||
| labels: | ||
| app: nginx | ||
| applications.argoproj.io/app-name: nginx | ||
| spec: | ||
| containers: | ||
| - image: 'nginx:1.23.1' | ||
| imagePullPolicy: Never | ||
| livenessProbe: | ||
| exec: | ||
| command: | ||
| - cat | ||
| - non-existent-file | ||
| failureThreshold: 3 | ||
| initialDelaySeconds: 5 | ||
| periodSeconds: 180 | ||
| successThreshold: 1 | ||
| timeoutSeconds: 1 | ||
| name: nginx | ||
| ports: | ||
| - containerPort: 80 | ||
| protocol: TCP | ||
| - containerPort: 8080 | ||
| protocol: TCP | ||
| - containerPort: 8081 | ||
| protocol: UDP | ||
| resources: | ||
| requests: | ||
| memory: 512Mi | ||
| cpu: 500m | ||
| terminationMessagePath: /dev/termination-log | ||
| terminationMessagePolicy: File | ||
| dnsPolicy: ClusterFirst | ||
| restartPolicy: Always | ||
| schedulerName: default-scheduler | ||
| securityContext: {} | ||
| terminationGracePeriodSeconds: 30 | ||
| status: | ||
| availableReplicas: 2 | ||
| conditions: | ||
| - lastTransitionTime: '2022-09-18T23:50:25Z' | ||
| lastUpdateTime: '2022-09-18T23:50:26Z' | ||
| message: ReplicaSet "nginx-deployment-6d68ff5f86" has successfully progressed. | ||
| reason: NewReplicaSetAvailable | ||
| status: 'True' | ||
| type: Progressing | ||
| - lastTransitionTime: '2022-09-23T18:30:59Z' | ||
| lastUpdateTime: '2022-09-23T18:30:59Z' | ||
| message: Deployment has minimum availability. | ||
| reason: MinimumReplicasAvailable | ||
| status: 'True' | ||
| type: Available | ||
| observedGeneration: 1 | ||
| readyReplicas: 2 | ||
| replicas: 2 | ||
| updatedReplicas: 2 |
Oops, something went wrong.