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

Remove BOMRepo and CompatibilityFilePath dependency from ClientConfig #3639

Merged
merged 1 commit into from
Oct 18, 2022

Conversation

anujc25
Copy link
Contributor

@anujc25 anujc25 commented Oct 12, 2022

What this PR does / why we need it

For BomRepo and CompatibilityFilePath fields in ClientConfig, the cluster and management-cluster plugins are not reading on this information from ClientConfig. However, plugins are configuring this fields when invoking the commands.

Reference: As part of PR #3264 we removed the dependency of plugins on ClientConfig.

Also, core CLI is always making sure that this fields are configured in the ClientConfig to support older plugins.

So, This PR removes the SetCompatibilityFileBasedOnEdition invocation as part of plugin code and marks fields as Deprecated.

Which issue(s) this PR fixes

Related to #3149

Describe testing done for PR

Release note

Remove `BOMRepo` and `CompatibilityFilePath` dependency from ClientConfig

Additional information

Special notes for your reviewer

@anujc25 anujc25 requested review from a team as code owners October 12, 2022 19:32
@anujc25 anujc25 requested a review from tenczar October 12, 2022 19:33
@github-actions
Copy link

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

@anujc25 anujc25 force-pushed the remove-edition-bom-dep branch from 79bb89e to f863b3d Compare October 12, 2022 23:09
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3639/20221012232714/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 Oct 13, 2022

Codecov Report

Merging #3639 (9b6294f) into main (71d32bc) will decrease coverage by 0.91%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #3639      +/-   ##
==========================================
- Coverage   46.21%   45.30%   -0.92%     
==========================================
  Files         400      425      +25     
  Lines       39624    41180    +1556     
==========================================
+ Hits        18314    18655     +341     
- Misses      19618    20817    +1199     
- Partials     1692     1708      +16     
Impacted Files Coverage Δ
...runtime/apis/config/v1alpha1/clientconfig_types.go 100.00% <ø> (ø)
cmd/cli/plugin/cluster/create.go 42.22% <ø> (ø)
cmd/cli/plugin/cluster/upgrade.go 58.50% <ø> (ø)
tkg/tkgctl/compatibility.go 0.00% <0.00%> (-70.00%) ⬇️
cli/runtime/config/conversion.go 76.19% <0.00%> (-2.66%) ⬇️
cli/runtime/component/prompt.go 67.46% <0.00%> (-0.77%) ⬇️
cmd/cli/plugin/cluster/scale.go 17.85% <0.00%> (ø)
...cluster/delete_machinehealthcheck_control_plane.go 16.66% <0.00%> (ø)
cmd/cli/plugin/cluster/get.go 6.27% <0.00%> (ø)
cmd/cli/plugin/cluster/credentials_update.go 9.23% <0.00%> (ø)
... and 23 more

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

Copy link
Contributor

@tenczar tenczar left a comment

Choose a reason for hiding this comment

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

One small comment, but sounds good to me.

cli/runtime/apis/config/v1alpha1/clientconfig_types.go Outdated Show resolved Hide resolved
…Config

Signed-off-by: Anuj Chaudhari <anujc@vmware.com>
@anujc25 anujc25 force-pushed the remove-edition-bom-dep branch from f3c8f6a to 9b6294f Compare October 17, 2022 20:07
@github-actions
Copy link

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

@tenczar tenczar added the ok-to-merge PRs should be labelled with this before merging label Oct 18, 2022
@anujc25 anujc25 merged commit 93a77e7 into vmware-tanzu:main Oct 18, 2022
jeffwubj pushed a commit that referenced this pull request Oct 24, 2022
…Config (#3639)

Signed-off-by: Anuj Chaudhari <anujc@vmware.com>

Signed-off-by: Anuj Chaudhari <anujc@vmware.com>
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
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.

3 participants