upgrade command add three-way-merge option #304

luxurine · 2021-09-02T08:37:30Z

upgrade command add three-way-merge option to show diff for actual state vs desired state #176

prepared resource for testing

1、last release:

---
# Source: base-app/templates/service.yaml
apiVersion: v1
kind: Service
metadata:
  name: base-app-go
  labels:
    app: base-app-go
    team: ops
    owner: somebody
    helm.sh/chart: base-app-1.0.0
    app.kubernetes.io/managed-by: Helm
spec:
  type: ClusterIP
  ports:
    - port: 1323
      targetPort: 1323
      protocol: TCP
      name: http-1323
  selector:
    app: base-app-go
---
# Source: base-app/templates/deployment.yaml
apiVersion: apps/v1
kind: Deployment
metadata:
  name: base-app-go
  labels:
    app: base-app-go
    team: ops
    owner: somebody
    helm.sh/chart: base-app-1.0.0
    app.kubernetes.io/managed-by: Helm
spec:
  replicas: 1
  selector:
    matchLabels:
      app: base-app-go
  strategy:
    type: RollingUpdate
  template:
    metadata:
      labels:
        app: base-app-go
        team: ops
        owner: somebody
        helm.sh/chart: base-app-1.0.0
        app.kubernetes.io/managed-by: Helm
    spec:
      imagePullSecrets:
        - name: regcred
      serviceAccountName: default
      initContainers:
      containers:
        - name: base-app-go
          image: xxxxx
          imagePullPolicy: IfNotPresent
          ports:
            - name: http-1323
              containerPort: 1323
              protocol: TCP
          lifecycle:
            preStop:
              exec:
                command:
                  - '/bin/sh'
                  - '-c'
                  - 'sleep 5'
          livenessProbe:
            httpGet:
              path: /healthz
              port: 1323
            initialDelaySeconds: 10
            periodSeconds: 60
          readinessProbe:
            httpGet:
              path: /healthz
              port: 1323
            initialDelaySeconds: 10
            periodSeconds: 3
          volumeMounts:
            - mountPath: /etc/localtime
              name: tz-config
              readOnly: true
            - mountPath: /sock
              name: sock-files
          envFrom:
          env:
            - name: CLUSTER
              value: dev
            - name: COUNT
              value: "3"
      volumes:
        - hostPath:
            path: /etc/localtime
            type: ""
          name: tz-config
        - emptyDir: {}
          name: sock-files

2、target release:

---
# Source: base-app/templates/service.yaml
apiVersion: v1
kind: Service
metadata:
  name: base-app-go
  labels:
    app: base-app-go
    team: ops
    owner: somebody
    helm.sh/chart: base-app-1.0.0
    app.kubernetes.io/managed-by: Helm
spec:
  type: ClusterIP
  ports:
    - port: 1323
      targetPort: 1323
      protocol: TCP
      name: http-1323
  selector:
    app: base-app-go
---
# Source: base-app/templates/deployment.yaml
apiVersion: apps/v1
kind: Deployment
metadata:
  name: base-app-go
  labels:
    app: base-app-go
    team: ops
    owner: somebody
    helm.sh/chart: base-app-1.0.0
    app.kubernetes.io/managed-by: Helm
spec:
  replicas: 1
  selector:
    matchLabels:
      app: base-app-go
  strategy:
    type: RollingUpdate
  template:
    metadata:
      labels:
        app: base-app-go
        team: ops
        owner: somebody
        helm.sh/chart: base-app-1.0.0
        app.kubernetes.io/managed-by: Helm
    spec:
      imagePullSecrets:
        - name: regcred
      serviceAccountName: default
      initContainers:
      containers:
        - name: base-app-go
          image: xxxxx
          imagePullPolicy: IfNotPresent
          ports:
            - name: http-1323
              containerPort: 1323
              protocol: TCP
          lifecycle:
            preStop:
              exec:
                command:
                  - '/bin/sh'
                  - '-c'
                  - 'sleep 5'
          livenessProbe:
            initialDelaySeconds: 10
            periodSeconds: 60
            tcpSocket:
              port: 1323
          readinessProbe:
            httpGet:
              path: /healthz
              port: 1323
            initialDelaySeconds: 10
            periodSeconds: 3
          volumeMounts:
            - mountPath: /etc/localtime
              name: tz-config
              readOnly: true
            - mountPath: /sock
              name: sock-files
          envFrom:
          env:
            - name: CLUSTER
              value: dev
            - name: COUNT
              value: "4"
      volumes:
        - hostPath:
            path: /etc/localtime
            type: ""
          name: tz-config
        - emptyDir: {}
          name: sock-files

normal diff vs diff with `--three-way-merge` option

1、manual change：

preStop time from 5 to 10
delete readinessProbe
delete Service

2、normal diff output:

default, base-app-go, Deployment (apps) has changed:
...
                    - '-c'
                    - 'sleep 5'
            livenessProbe:
-             httpGet:
-               path: /healthz
-               port: 1323
              initialDelaySeconds: 10
              periodSeconds: 60
+             tcpSocket:
+               port: 1323
            readinessProbe:
              httpGet:
                path: /healthz
...
              - name: CLUSTER
                value: dev
              - name: COUNT
-               value: "3"
+               value: "4"
        volumes:
          - hostPath:
              path: /etc/localtime

3、diff output with --three-way-merge option:

default, base-app-go, Deployment (apps) has changed:
...
      meta.helm.sh/release-name: base-app-01
      meta.helm.sh/release-namespace: default
    creationTimestamp: "2021-09-01T06:29:39Z"
-   generation: 8
+   generation: 9
    labels:
      app: base-app-go
      app.kubernetes.io/managed-by: Helm
...
       - env:
          - name: CLUSTER
             value: dev
          - name: COUNT
-           value: "3"
+           value: "4"
          image: "xxxxx"
          imagePullPolicy: IfNotPresent
          lifecycle:
...
                command:
                - /bin/sh
                - -c
-               - sleep 10
+               - sleep 5
          livenessProbe:
            failureThreshold: 3
-           httpGet:
-             path: /healthz
-             port: 1323
-             scheme: HTTP
            initialDelaySeconds: 10
            periodSeconds: 60
            successThreshold: 1
+           tcpSocket:
+             port: 1323
            timeoutSeconds: 1
          name: base-app-go
          ports:
          - containerPort: 1323
            name: http-1323
            protocol: TCP
+         readinessProbe:
+           failureThreshold: 3
+           httpGet:
+             path: /healthz
+             port: 1323
+             scheme: HTTP
+           initialDelaySeconds: 10
+           periodSeconds: 3
+           successThreshold: 1
+           timeoutSeconds: 1
          resources: {}
          terminationMessagePath: /dev/termination-log
          terminationMessagePolicy: File
...
default, base-app-go, Service (v1) has been added:
- 
+ apiVersion: v1
+ kind: Service
+ metadata:
+   labels:
+     app: base-app-go
+     app.kubernetes.io/managed-by: Helm
+     helm.sh/chart: base-app-1.0.0
+     owner: somebody
+     team: ops
+   name: base-app-go
+   namespace: default
+ spec:
+   ports:
+   - name: http-1323
+     port: 1323
+     protocol: TCP
+     targetPort: 1323
+   selector:
+     app: base-app-go
+   type: ClusterIP

inspired by https://github.com/bacongobbler/helm-patchdiff

…ate vs desired state

Temikus · 2021-09-21T09:23:45Z

@databus23 @mumoshu can I humbly ask you folks to take a look? 🙏

This might be a step in the right direction to solve #176

stale · 2022-01-03T21:21:10Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

mumoshu · 2022-01-08T08:36:38Z

cmd/upgrade.go

@@ -169,6 +192,25 @@ func (d *diffCmd) runHelm3() error {
 		return fmt.Errorf("Failed to render chart: %s", err)
 	}

+	if d.threeWayMerge {


Probably L171-L193 doesn't need to be run when we enter the 3-way merge mode?

Nevermind. It turned out correct as we still render the chart at L190 to be used when building K8s objects at L207.

mumoshu

I have one concern but this looks great for a starter. Thanks a lot for your patience and contribution!

dudicoco · 2022-01-09T10:41:06Z

Thank you for this addition @luxurine and @mumoshu

When can we expect a new release which contains this change?

This adds a dedicated envvar to enable the three-way-merge diff added in #304. It should be useful when you are using helm-diff from another command or script and you dont have ability to specify the --three-way-merge flag without modifying the command/script but you cant due to various reasons.

mumoshu · 2022-01-09T12:09:59Z

Yeah maybe in coming weeks.

mumoshu · 2022-01-09T12:10:24Z

FYI, I've added a feature to enable this via an envvar. See #336 for more information.

upgrade command add three-way-merge option to show diff for actual st…

e01ed6e

…ate vs desired state

dylanbannon mentioned this pull request Dec 20, 2021

adding manifest=true to all helm providers cloudposse/terraform-aws-components#381

Closed

stale bot added the stale label Jan 3, 2022

mumoshu removed the stale label Jan 8, 2022

mumoshu reviewed Jan 8, 2022

View reviewed changes

mumoshu approved these changes Jan 8, 2022

View reviewed changes

mumoshu merged commit 494eef0 into databus23:master Jan 8, 2022

mumoshu mentioned this pull request Jan 9, 2022

HELM_DIFF_THREE_WAY_MERGE=true to imply --three-way-merge #336

Merged

mumoshu mentioned this pull request Jan 9, 2022

Failure to honor lookup function #263

Closed

mumoshu mentioned this pull request Feb 13, 2022

How to validate if K8s secrets are base64 encoded during helm diff? #366

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

upgrade command add three-way-merge option #304

upgrade command add three-way-merge option #304

luxurine commented Sep 2, 2021 •

edited

Loading

Temikus commented Sep 21, 2021

stale bot commented Jan 3, 2022

mumoshu Jan 8, 2022

mumoshu Jan 9, 2022 •

edited

Loading

mumoshu left a comment

dudicoco commented Jan 9, 2022 •

edited

Loading

mumoshu commented Jan 9, 2022

mumoshu commented Jan 9, 2022

upgrade command add three-way-merge option #304

upgrade command add three-way-merge option #304

Conversation

luxurine commented Sep 2, 2021 • edited Loading

upgrade command add three-way-merge option to show diff for actual state vs desired state #176

prepared resource for testing

normal diff vs diff with --three-way-merge option

Temikus commented Sep 21, 2021

stale bot commented Jan 3, 2022

mumoshu Jan 8, 2022

Choose a reason for hiding this comment

mumoshu Jan 9, 2022 • edited Loading

Choose a reason for hiding this comment

mumoshu left a comment

Choose a reason for hiding this comment

dudicoco commented Jan 9, 2022 • edited Loading

mumoshu commented Jan 9, 2022

mumoshu commented Jan 9, 2022

luxurine commented Sep 2, 2021 •

edited

Loading

normal diff vs diff with `--three-way-merge` option

mumoshu Jan 9, 2022 •

edited

Loading

dudicoco commented Jan 9, 2022 •

edited

Loading