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

Fix fallback creation with legacy style cluster #2898

Merged
merged 2 commits into from
Jul 12, 2022
Merged

Conversation

vuil
Copy link
Contributor

@vuil vuil commented Jul 11, 2022

What this PR does / why we need it

When deem necessary to create cluster using legacy approach, said creation should work.
Also:

  • added integration tests to verify that the cluster creation works under such scenario.

Which issue(s) this PR fixes

Fixes #2830

Describe testing done for PR

See provided integration tests

Release note

package-based-lcm: when feature flag is set, legacy style cluster creation should still work

PR Checklist

  • x Squash the commits into one or a small number of logical commits
  • x Use good commit messages
  • x Ensure PR contains terms all contributors can understand and links all contributors can access

Additional information

Special notes for your reviewer

Tests had to work around an issue #2891
by prepopulating a tkr bom config map. Said workaround should be removed as soon as the issue is fixed.

With bifuraction of cc and non-cc based ytt overlays,
non-cc overlays should behave identically regardless of
whether the pblcm feature flag is set or not.

Fixes: #2830

Signed-off-by: Vui Lam <vui@vmware.com>
@vuil vuil requested review from a team as code owners July 11, 2022 15:34
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/2898/20220711154346/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.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/2898/20220711161309/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.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/2898/20220711221050/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.

@codecov
Copy link

codecov bot commented Jul 11, 2022

Codecov Report

Merging #2898 (9f86a30) into main (447cacf) will increase coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2898      +/-   ##
==========================================
+ Coverage   43.83%   43.87%   +0.03%     
==========================================
  Files         414      414              
  Lines       41305    41316      +11     
==========================================
+ Hits        18105    18126      +21     
+ Misses      21507    21499       -8     
+ Partials     1693     1691       -2     
Impacted Files Coverage Δ
...ons/controllers/packageinstallstatus_controller.go 80.64% <0.00%> (-2.16%) ⬇️
pkg/v1/tkg/client/get_cluster.go 54.16% <0.00%> (ø)
addons/controllers/clusterbootstrap_controller.go 63.54% <0.00%> (+0.26%) ⬆️
cmd/cli/plugin/tkr/v1alpha3/os.go 74.35% <0.00%> (+0.85%) ⬆️
pkg/v1/tkg/tkgctl/get_cluster.go 86.66% <0.00%> (+7.71%) ⬆️
...oller-manager/controllers/v3_cascade_controller.go 73.33% <0.00%> (+11.42%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 447cacf...9f86a30. Read the comment docs.

Copy link
Contributor

@anujc25 anujc25 left a comment

Choose a reason for hiding this comment

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

Looks good.
Just one minor question.

@vuil vuil added area/cc ClusterClass related area/lcm Related to Cluster Lifecycle management ok-to-merge PRs should be labelled with this before merging labels Jul 12, 2022
- Fixes incorrect plan name in aws_cc_antrea_test.go.
- Adds test to verify that legacy style creation is used when modified
  overlays are detected.
- Includes some cleanups and refactoring.

Signed-off-by: Vui Lam <vui@vmware.com>
@vuil vuil merged commit 0e8e99f into main Jul 12, 2022
@vuil vuil deleted the topic/fallback-2830-fix branch July 12, 2022 15:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/cc ClusterClass related area/lcm Related to Cluster Lifecycle management cla-not-required ok-to-merge PRs should be labelled with this before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Creation of workload cluster using legacy config file fails when package-based-lcm feature flag is activated
3 participants