Move other values to global #423

nprokopic · 2023-11-17T16:04:58Z

⚠️ This is a Helm values breaking change, so it will require adjusting Helm values for all cluster when doing the upgrade.

What this PR does / why we need it

We have ported all provider-independent Cluster API resources to cluster chart, which was phase 1 of restructuring of cluster- apps, see giantswarm/roadmap#2742 for more details. Now we want to use cluster chart in cluster-aws and remove all provider-independent Cluster API resources from cluster-aws.

In order to do so, first we have to refactor cluster-aws Helm values, so that cluster chart can read provider-independent values it needs. For that, we have to move current top-level properties to be under Values.global.

This pull request makes the following changes to Helm values:

Move Helm values property .Values.managementCluster to .Values.global.managementCluster.
Move Helm values property .Values.baseDomain to .Values.global.connectivity.baseDomain.

Checklist

Update changelog in CHANGELOG.md.

Trigger e2e tests

/run cluster-test-suites

github-actions · 2023-11-17T16:06:28Z

(helm/cluster-aws/ci/test-wc-minimal-values.yaml) rendered manifest diff

/metadata/labels/helm.sh/chart  (AWSCluster/org-giantswarm/test-wc-minimal)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/metadata/labels/app.kubernetes.io/version  (AWSCluster/org-giantswarm/test-wc-minimal)
  ± value change
    - 0.48.1
    + 0.47.0

/metadata/labels/helm.sh/chart  (AWSMachinePool/org-giantswarm/test-wc-minimal-def00)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/metadata/labels/app.kubernetes.io/version  (AWSMachinePool/org-giantswarm/test-wc-minimal-def00)
  ± value change
    - 0.48.1
    + 0.47.0

/metadata/labels/helm.sh/chart  (AWSMachineTemplate/org-giantswarm/test-wc-minimal-control-plane-779bfa58)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/metadata/labels/app.kubernetes.io/version  (AWSMachineTemplate/org-giantswarm/test-wc-minimal-control-plane-779bfa58)
  ± value change
    - 0.48.1
    + 0.47.0

/spec/template/metadata/labels/helm.sh/chart  (AWSMachineTemplate/org-giantswarm/test-wc-minimal-control-plane-779bfa58)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/metadata/labels/helm.sh/chart  (AWSMachineTemplate/org-giantswarm/test-wc-minimal-bastion-17f76896)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/metadata/labels/app.kubernetes.io/version  (AWSMachineTemplate/org-giantswarm/test-wc-minimal-bastion-17f76896)
  ± value change
    - 0.48.1
    + 0.47.0

/spec/template/metadata/labels/helm.sh/chart  (AWSMachineTemplate/org-giantswarm/test-wc-minimal-bastion-17f76896)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/metadata/labels/helm.sh/chart  (Cluster/org-giantswarm/test-wc-minimal)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/metadata/labels/app.kubernetes.io/version  (Cluster/org-giantswarm/test-wc-minimal)
  ± value change
    - 0.48.1
    + 0.47.0

/metadata/labels/helm.sh/chart  (HelmRelease/org-giantswarm/test-wc-minimal-aws-ebs-csi-driver)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/metadata/labels/helm.sh/chart  (HelmRelease/org-giantswarm/test-wc-minimal-cilium)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/spec/chart/spec/version  (HelmRelease/org-giantswarm/test-wc-minimal-cilium)
  ± value change
    - 0.17.0
    + 0.13.0

/metadata/labels/helm.sh/chart  (HelmRelease/org-giantswarm/test-wc-minimal-cloud-provider-aws)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/metadata/labels/helm.sh/chart  (HelmRelease/org-giantswarm/test-wc-minimal-coredns)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/metadata/labels/helm.sh/chart  (HelmRelease/org-giantswarm/test-wc-minimal-vertical-pod-autoscaler-crd)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/metadata/labels/helm.sh/chart  (HelmRepository/org-giantswarm/test-wc-minimal-default)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/metadata/labels/helm.sh/chart  (HelmRepository/org-giantswarm/test-wc-minimal-default-test)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/metadata/labels/helm.sh/chart  (KubeadmConfig/org-giantswarm/test-wc-minimal-def00)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/metadata/labels/app.kubernetes.io/version  (KubeadmConfig/org-giantswarm/test-wc-minimal-def00)
  ± value change
    - 0.48.1
    + 0.47.0

/metadata/labels/helm.sh/chart  (KubeadmConfigTemplate/org-giantswarm/test-wc-minimal-bastion-c7cb92f0)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/metadata/labels/helm.sh/chart  (KubeadmControlPlane/org-giantswarm/test-wc-minimal)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/metadata/labels/app.kubernetes.io/version  (KubeadmControlPlane/org-giantswarm/test-wc-minimal)
  ± value change
    - 0.48.1
    + 0.47.0

/spec/machineTemplate/metadata/labels/helm.sh/chart  (KubeadmControlPlane/org-giantswarm/test-wc-minimal)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/spec/machineTemplate/metadata/labels/app.kubernetes.io/version  (KubeadmControlPlane/org-giantswarm/test-wc-minimal)
  ± value change
    - 0.48.1
    + 0.47.0

/metadata/labels/helm.sh/chart  (MachineDeployment/org-giantswarm/test-wc-minimal-bastion)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/metadata/labels/app.kubernetes.io/version  (MachineDeployment/org-giantswarm/test-wc-minimal-bastion)
  ± value change
    - 0.48.1
    + 0.47.0

/spec/template/metadata/labels/helm.sh/chart  (MachineDeployment/org-giantswarm/test-wc-minimal-bastion)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/metadata/labels/helm.sh/chart  (MachineHealthCheck/org-giantswarm/test-wc-minimal-control-plane)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/metadata/labels/app.kubernetes.io/version  (MachineHealthCheck/org-giantswarm/test-wc-minimal-control-plane)
  ± value change
    - 0.48.1
    + 0.47.0

/metadata/labels/helm.sh/chart  (MachinePool/org-giantswarm/test-wc-minimal-def00)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/metadata/labels/app.kubernetes.io/version  (MachinePool/org-giantswarm/test-wc-minimal-def00)
  ± value change
    - 0.48.1
    + 0.47.0

/metadata/labels/helm.sh/chart  (ServiceAccount/org-giantswarm/test-wc-minimal-cleanup-helmreleases-hook)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/metadata/labels/helm.sh/chart  (Role/org-giantswarm/test-wc-minimal-cleanup-helmreleases-hook)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/metadata/labels/helm.sh/chart  (RoleBinding/org-giantswarm/test-wc-minimal-cleanup-helmreleases-hook)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/metadata/labels/helm.sh/chart  (Job/org-giantswarm/test-wc-minimal-cleanup-helmreleases-hook)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

/spec/template/metadata/labels/helm.sh/chart  (Job/org-giantswarm/test-wc-minimal-cleanup-helmreleases-hook)
  ± value change
    - cluster-aws-0.48.1
    + cluster-aws-0.47.0

helm/cluster-aws/values.schema.json

AndiDog

Where do the new values get filled in? I guess baseDomain comes from cluster-apps-operator but for managementCluster I forgot the config source.

The new .Values.x.y.z paths should be required via JSON schema, given how we use them in the manifest without {{ required ... }}, assuming non-empty values.

fiunchinho · 2023-11-28T16:23:29Z

Where do the new values get filled in? I guess baseDomain comes from cluster-apps-operator but for managementCluster I forgot the config source.

From the app catalog https://github.com/giantswarm/giantswarm-management-clusters/pull/289

Temporary until all clusters are upgraded

tinkerers-ci · 2023-11-30T00:44:47Z

cluster-test-suites

Run name	`pr-cluster-aws-423-cluster-test-suites8z57h`
Commit SHA	`de96379`
Result	Failed ❌

📋 View full results in Tekton Dashboard

Rerun trigger:
/run cluster-test-suites

nprokopic mentioned this pull request Nov 17, 2023

Implementing Phase 1, part 2/5: Refactor cluster-aws Helm values giantswarm/roadmap#2954

Closed

nprokopic marked this pull request as ready for review November 23, 2023 16:53

nprokopic requested a review from a team as a code owner November 23, 2023 16:53

nprokopic requested a review from a team November 23, 2023 16:53

njuettner approved these changes Nov 23, 2023

View reviewed changes

njuettner reviewed Nov 23, 2023

View reviewed changes

helm/cluster-aws/values.schema.json Show resolved Hide resolved

nprokopic force-pushed the move-nodepools-to-global branch from 4c832cb to e0ab346 Compare November 28, 2023 14:37

nprokopic force-pushed the move-other-values-to-global branch from 7542ea1 to c3790e5 Compare November 28, 2023 14:37

fiunchinho approved these changes Nov 28, 2023

View reviewed changes

nprokopic force-pushed the move-nodepools-to-global branch from e0ab346 to 8fda797 Compare November 28, 2023 14:49

nprokopic force-pushed the move-other-values-to-global branch 2 times, most recently from 3fb78f0 to e9e35e3 Compare November 28, 2023 15:13

AndiDog reviewed Nov 28, 2023

View reviewed changes

AndiDog approved these changes Nov 29, 2023

View reviewed changes

nprokopic force-pushed the move-nodepools-to-global branch from 273a64a to 8a87880 Compare November 29, 2023 23:38

nprokopic force-pushed the move-other-values-to-global branch from e9e35e3 to e11607b Compare November 29, 2023 23:39

nprokopic force-pushed the move-nodepools-to-global branch from 8a87880 to 2e52982 Compare November 29, 2023 23:59

nprokopic force-pushed the move-other-values-to-global branch from e11607b to 171f174 Compare November 30, 2023 00:00

nprokopic force-pushed the move-nodepools-to-global branch from 2e52982 to 23eaec1 Compare November 30, 2023 00:08

nprokopic force-pushed the move-other-values-to-global branch from 171f174 to de96379 Compare November 30, 2023 00:09

Base automatically changed from move-nodepools-to-global to master November 30, 2023 00:12

nprokopic added 6 commits November 30, 2023 01:13

Move .Values.managementCluster to .Values.global.managementCluster

43638d1

Correctly render Helm values docs

7177387

Move property Values.baseDomain to Values.global.connectivity.baseDomain

55c58c5

Allow baseDomain and managementCluster properties in root-level as well

79644eb

Temporary until all clusters are upgraded

Update docs

fd785f8

Fix CI values

4700041

nprokopic force-pushed the move-other-values-to-global branch from de96379 to 4700041 Compare November 30, 2023 00:15

nprokopic merged commit d2b6b87 into master Nov 30, 2023
14 of 15 checks passed

nprokopic deleted the move-other-values-to-global branch November 30, 2023 00:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move other values to global #423

Move other values to global #423

nprokopic commented Nov 17, 2023

github-actions bot commented Nov 17, 2023

AndiDog left a comment

fiunchinho commented Nov 28, 2023

tinkerers-ci bot commented Nov 30, 2023

Move other values to global #423

Move other values to global #423

Conversation

nprokopic commented Nov 17, 2023

What this PR does / why we need it

Checklist

Trigger e2e tests

github-actions bot commented Nov 17, 2023

AndiDog left a comment

Choose a reason for hiding this comment

fiunchinho commented Nov 28, 2023

tinkerers-ci bot commented Nov 30, 2023

cluster-test-suites