search

rancher / charts-build-scripts

Apache License 2.0
9 stars 32 forks source link

Fix ./scripts/package-ci to respect patches generated from different `diff` versions #90

Open aiyengar2 opened 1 year ago

aiyengar2 commented 1 year ago

Currently, ./scripts/package-ci is used in order to manually verify whether an upgrade to rancher/charts-build-scripts will break a charts-build-scripts repository.

It does this by running make prepare, make patch, make clean, and asserting no changes are found.

However, when two different machines are used to run make patch today, it's possible that both can produce equally valid patches that are slightly different due to the way each machine chooses to construct the patch. For example, the following failures are noted in rancher/charts today:

Failed to pass ci for the following packages:
- epinio/epinio-ui generated additional changes: M packages/epinio/epinio-ui/generated-changes/patch/templates/_helpers.tpl.patch
- harvester/harvester-cloud-provider generated additional changes: M packages/harvester/harvester-cloud-provider/generated-changes/patch/Chart.yaml.patch
- harvester/harvester-csi-driver generated additional changes: M packages/harvester/harvester-csi-driver/generated-changes/patch/Chart.yaml.patch
- longhorn/longhorn-1.2 generated additional changes: M packages/longhorn/longhorn-1.2/generated-changes/patch/README.md.patch
- longhorn/longhorn-1.3 generated additional changes: M packages/longhorn/longhorn-1.3/generated-changes/patch/README.md.patch
- neuvector generated additional changes: M packages/neuvector/generated-changes/overlay/crds/_helpers.tpl
- rancher-alerting/rancher-prom2teams generated additional changes: M packages/rancher-alerting/rancher-prom2teams/generated-changes/patch/templates/_helpers.tpl.patch M packages/rancher-alerting/rancher-prom2teams/generated-changes/patch/templates/deployment.yaml.patch
- rancher-gatekeeper generated additional changes: M packages/rancher-gatekeeper/generated-changes/patch/templates/_helpers.tpl.patch
- rancher-istio/1.14/rancher-kiali-server generated additional changes: M packages/rancher-istio/1.14/rancher-kiali-server/generated-changes/patch/templates/_helpers.tpl.patch
- rancher-monitoring/rancher-grafana generated additional changes: M packages/rancher-monitoring/rancher-grafana/generated-changes/patch/templates/tests/test-podsecuritypolicy.yaml.patch
- rancher-monitoring/rancher-node-exporter generated additional changes: M packages/rancher-monitoring/rancher-node-exporter/generated-changes/patch/templates/daemonset.yaml.patch M packages/rancher-monitoring/rancher-node-exporter/generated-changes/patch/values.yaml.patch

But on inspecting the changes, both patches are valid; i.e. here is the diff on the diff.

diff --git a/packages/longhorn/longhorn-1.2/generated-changes/patch/README.md.patch b/packages/longhorn/longhorn-1.2/generated-changes/patch/README.md.patch
index b92a6e20c..ce9896912 100644
--- a/packages/longhorn/longhorn-1.2/generated-changes/patch/README.md.patch
+++ b/packages/longhorn/longhorn-1.2/generated-changes/patch/README.md.patch
@@ -34,14 +34,14 @@
 -helm delete longhorn --purge
 -```
 +To prevent damage to the Kubernetes cluster, we recommend deleting all Kubernetes workloads using Longhorn volumes (PersistentVolume, PersistentVolumeClaim, StorageClass, Deployment, StatefulSet, DaemonSet, etc).
-+
-+From Rancher Cluster Explorer UI, navigate to Apps page, delete app `longhorn` then app `longhorn-crd` in Installed Apps tab.

 -With Helm 3 to uninstall Longhorn.
 -```
 -helm uninstall longhorn -n longhorn-system
 -kubectl delete namespace longhorn-system
 -```
++From Rancher Cluster Explorer UI, navigate to Apps page, delete app `longhorn` then app `longhorn-crd` in Installed Apps tab.
++

  ---
  Please see [link](https://github.com/longhorn/longhorn) for more information.

It is equally valid here to add From Rancher Cluster Explorer UI... before or after removing With Helm 3 to uninstall Longhorn, so both patches are logically the same but are failing CI.

To fix this, one of three approaches should be taken:

  1. We should ensure that the output of make patch is the same across all machines by avoiding using the system call to run diff -uN to generate these patches: https://github.com/rancher/charts-build-scripts/blob/2901446ef556324ce10bdf88889181516d45dd08/pkg/diff/diff.go#L25
  2. We should ensure that the output of make patch is the same across all machines by containerizing rancher/charts-build-scripts runs; to do this, we'd need to avoid using an osfs (https://github.com/rancher/charts-build-scripts/blob/2901446ef556324ce10bdf88889181516d45dd08/pkg/filesystem/filesystem.go#L24) and remove all os calls in favor of a memfs https://github.com/go-git/go-billy/blob/205ba6585ca03bd60e304f7fa0b270232ba1d707/memfs/memory.go#L28-L31
  3. We should refactor the logic of the package-ci script to avoid comparing make patch and instead compare the output of make charts to ensure the underlying chart that is generated has not been changed.