Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Add logic to put cluster labels and package labels on resources for custom cluster bootstrap scenario #4544

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

shivaani0505
Copy link
Contributor

@shivaani0505 shivaani0505 commented Apr 3, 2023

What this PR does / why we need it

  1. Add logic to put cluster labels and package labels(tkg.tanzu.vmware.com/package-name) on resources for custom cluster bootstrap scenario

  2. Simplify the logic of reconciliation of existing cluster bootstrap in HandleExistingClusterBootstrap and CloneReferencedObjectsFromCBPackages by making cloneSecretRef and cloneProviderRef functions idempotent

  3. Add check for package name labels on config resources for Calico test and load balancer and ingress (AKO) test

Which issue(s) this PR fixes

Fixes #4568

Describe testing done for PR

  1. All unit tests pass on local
  2. Also fixes CAPA pipeline integration tests

Release note

Add logic to put cluster labels and package labels(tkg.tanzu.vmware.com/package-name) on resources for custom cluster bootstrap scenario

Additional information

Special notes for your reviewer

@github-actions
Copy link

github-actions bot commented Apr 3, 2023

CVE Scan results for this PR can be viewed from
https://github.com/vmware-tanzu/tanzu-framework/security/code-scanning?query=pr%3A4544

@github-advanced-security
Copy link

You have successfully added a new Trivy configuration .github/workflows/cve_scan.yaml:trivy_scan. As part of the setup process, we have scanned this repository and found no existing alerts. In the future, you will see all code scanning alerts on the repository Security tab.

@codecov
Copy link

codecov bot commented Apr 3, 2023

Codecov Report

Merging #4544 (c8c35d7) into main (bb07e03) will decrease coverage by 0.87%.
The diff coverage is 65.62%.

❗ Current head c8c35d7 differs from pull request most recent head c45cc8c. Consider uploading reports for the commit c45cc8c to get more accurate results

@@            Coverage Diff             @@
##             main    #4544      +/-   ##
==========================================
- Coverage   49.77%   48.91%   -0.87%     
==========================================
  Files         453      483      +30     
  Lines       45424    47505    +2081     
==========================================
+ Hits        22612    23238     +626     
- Misses      20652    22059    +1407     
- Partials     2160     2208      +48     
Impacted Files Coverage Δ
...til/clusterbootstrapclone/clusterbootstrapclone.go 67.58% <65.62%> (+1.45%) ⬆️

... and 35 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@shivaani0505
Copy link
Contributor Author

/test install-vc7

@shivaani0505
Copy link
Contributor Author

/test install-vc7,upgrade-vc7

@alfredthenarwhal
Copy link
Collaborator

@shivaani0505: /test install-vc7
Commit: 21177ba

Tests failed! Build no: 4695

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/4544/20230410215116/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@shivaani0505 shivaani0505 force-pushed the shivaani/refactor-provider-secret-ref-labels branch from d9e1a87 to 027f934 Compare April 11, 2023 22:38
@shivaani0505 shivaani0505 force-pushed the shivaani/refactor-provider-secret-ref-labels branch from 027f934 to c45cc8c Compare April 11, 2023 22:40
toBeCloned = false
for _, clonePackage := range packagesToBeCloned {
if cbPackage == clonePackage {
toBeCloned = true
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Wondering if we could break here (as more iterations of the loop won't change toBeCloned )?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants