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

[CAFV-464][1.3.z]Add suggested CRS labels to crs.yaml files #638

Merged
merged 4 commits into from
Apr 10, 2024

Conversation

rliang88
Copy link
Contributor

@rliang88 rliang88 commented Apr 3, 2024

Description

Please provide a brief description of the changes proposed in this Pull Request

Add these labels to all crs template files:

cluster-role.tkg.tanzu.vmware.com/management: ""
tanzuKubernetesRelease: ${TARGET_TANZUKUBERNETESRELEASE}
tkg.tanzu.vmware.com/cluster-name: ${CLUSTER_NAME}

Checklist

  • tested locally
  • updated any relevant dependencies
  • updated any relevant documentation or examples

API Changes

Are there API changes?

  • Yes
  • No

If yes, please fill in the below

  1. Updated conversions?
    • Yes
    • No
    • N/A
  2. Updated CRDs?
    • Yes
    • No
    • N/A
  3. Updated infrastructure-components.yaml?
    • Yes
    • No
    • N/A
  4. Updated ./examples/capi-quickstart.yaml?
    • Yes
    • No
    • N/A
  5. Updated necessary files under ./infrastructure-vcd/v1.0.0/?
    • Yes
    • No
    • N/A

Issue

If applicable, please reference the relevant issue

Fixes #


This change is Reviewable

Copy link
Contributor

@lzichong lzichong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 12 of 12 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @rliang88 and @sahithi)


templates/cluster-template-v1.20.8-crs.yaml line 11 at r1 (raw file):

    csi: external
    cluster-role.tkg.tanzu.vmware.com/management: ""
    tanzuKubernetesRelease: ${TARGET_TANZUKUBERNETESRELEASE}

Please check if clusterctl.yaml also needs to include TARGET_TANZUKUBERNETESRELEASE. When using clusterctl generate ..., it will look at clusterctl.yaml to set variables in the templates. Otherwise, if unset, it would give tanzuKubernetesRelease: "", if this is also okay we can move it as a optional field. Please double check this with @sahithi as well

:lgtm: otherwise.

@@ -57,7 +57,7 @@ ETCD_VERSION: v3.4.13_vmware.14 # Ignore this property if you are using one of t
DNS_VERSION: v1.7.0_vmware.12 # Ignore this property if you are using one of the existing flavors
POD_CIDR: "100.96.0.0/11"
SERVICE_CIDR: "100.64.0.0/13"
TKR_VERSION: v1.20.8---vmware.1-tkg.2 # Ignore this property if you are using one of the existing flavors
TARGET_TANZUKUBERNETESRELEASE: ""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry Richard, you did ask me whether to leave it empty and I said yes.
Can you please give a valid value so that users can see this as an example? See one of our RDEs that UI generates to see the format?

@rliang88 rliang88 merged commit 7712771 into vmware:1.3.z Apr 10, 2024
1 check was pending
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants