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

Detect TKGS environment for any version of TKC API #2970

Merged
merged 1 commit into from
Jul 27, 2022
Merged

Detect TKGS environment for any version of TKC API #2970

merged 1 commit into from
Jul 27, 2022

Conversation

abhijit-dev82
Copy link
Contributor

@abhijit-dev82 abhijit-dev82 commented Jul 19, 2022 •
edited by rajathagasthya
Loading

What this PR does / why we need it

Which issue(s) this PR fixes

Fixes #2786

Describe testing done for PR

Added a new method in capabilties_test.go

Release note

Issue: #2786
IsTKGS () API function might break because TKC v1alpha1 is likely to be dropped soon.

PR Checklist

Additional information

Special notes for your reviewer

@abhijit-dev82 abhijit-dev82 requested review from a team as code owners July 19, 2022 09:01
@github-actions
Copy link

Hi @abhijit-dev82! And thank you for opening your first Pull Request. Someone will review it soon. Thank you for committing to making Tanzu Framework better.

@codecov
Copy link

codecov bot commented Jul 19, 2022 •
edited
Loading

Codecov Report

Merging #2970 (32af7bf) into main (6d6ca50) will increase coverage by 0.14%.
The diff coverage is 100.00%.

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

@@            Coverage Diff             @@
##             main    #2970      +/-   ##
==========================================
+ Coverage   44.00%   44.15%   +0.14%     
==========================================
  Files         416      416              
  Lines       41852    42141     +289     
==========================================
+ Hits        18417    18607     +190     
- Misses      21714    21809      +95     
- Partials     1721     1725       +4
Impacted Files Coverage Δ
pkg/v1/sdk/capabilities/discovery/tkg/fake.go 57.14% <ø> (ø)
.../v1/sdk/capabilities/discovery/tkg/capabilities.go 76.08% <100.00%> (+5.63%) ⬆️
pkg/v1/tkg/tkgctl/featuregate_helper.go 19.35% <0.00%> (-35.20%) ⬇️
pkg/v2/tkr/util/version/version.go 87.20% <0.00%> (-2.08%) ⬇️
pkg/v1/tkg/client/scale.go 69.19% <0.00%> (-2.08%) ⬇️
...ons/controllers/packageinstallstatus_controller.go 79.15% <0.00%> (-1.50%) ⬇️
pkg/v1/config/defaults.go 42.30% <0.00%> (-0.83%) ⬇️
addons/controllers/clusterbootstrap_controller.go 63.28% <0.00%> (-0.62%) ⬇️
pkg/v1/tkg/clusterclient/clusterclient.go 48.73% <0.00%> (-0.35%) ⬇️
addons/controllers/addon_controller.go 63.30% <0.00%> (-0.31%) ⬇️
... and 15 more

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 6d6ca50...bd99430. Read the comment docs.

rajathagasthya
rajathagasthya reviewed Jul 19, 2022
View reviewed changes
Copy link
Member

@rajathagasthya rajathagasthya left a comment

Choose a reason for hiding this comment

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

Minor comments, but overall LGTM.

pkg/v1/sdk/capabilities/discovery/tkg/capabilities.go Show resolved Hide resolved
pkg/v1/sdk/capabilities/discovery/tkg/capabilities_test.go Show resolved Hide resolved
yharish991
yharish991 reviewed Jul 20, 2022
View reviewed changes
Copy link
Member

@yharish991 yharish991 left a comment

Choose a reason for hiding this comment

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

Added a minor comment, but this LGTM!!
Can you add the link to the issue this pr is addressing and release note in the pr description? Also, can you squash the commits, when you are done addressing the comments? thanks..

pkg/v1/sdk/capabilities/discovery/tkg/capabilities.go Outdated
@@ -38,7 +39,10 @@ func (dc *DiscoveryClient) IsTKGm(ctx context.Context) (bool, error) {

// IsTKGS returns true if the cluster is a TKGS cluster.
func (dc *DiscoveryClient) IsTKGS(ctx context.Context) (bool, error) {
return dc.HasTanzuKubernetesClusterV1alpha1(ctx)
/* Check for TKGS on any TKC API version */
Copy link
Member

Choose a reason for hiding this comment

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

can you make this a single line comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Taken care of the comment and squashed the commits.

@abhijit-dev82 abhijit-dev82 requested a review from yharish991 July 25, 2022 05:40
yharish991
yharish991 reviewed Jul 25, 2022
View reviewed changes
pkg/v1/sdk/capabilities/discovery/tkg/capabilities.go Outdated
@@ -36,9 +37,11 @@ func (dc *DiscoveryClient) IsTKGm(ctx context.Context) (bool, error) {
return dc.HasInfrastructureProvider(ctx, InfrastructureProviderVsphere)
}

// IsTKGS returns true if the cluster is a TKGS cluster.
// IsTKGS returns true if the cluster is a TKGS cluster. Check for TKGS on any TKC API version */
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// IsTKGS returns true if the cluster is a TKGS cluster. Check for TKGS on any TKC API version */
// IsTKGS returns true if the cluster is a TKGS cluster. Checks for the existence of any TKC API version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think the suggested change was committed?

@rajathagasthya
Copy link
Member

Could you please change the commit message to be more meaningful? Something like Look for any version of TKC API to detect TKGS env.

Also, please fix build/test issues (or rebase if it's unrelated) before this can be merged. Thanks.

@abhijit-dev82 abhijit-dev82 changed the title Modified capabilties API to a generic call Detect TKGS environment for any version of TKC API Jul 26, 2022
@abhijit-dev82 abhijit-dev82 requested review from a team as code owners July 27, 2022 04:29
@github-actions
Copy link

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

@abhijit-dev82 abhijit-dev82 requested review from a team as code owners July 27, 2022 05:05
@abhijit-dev82 abhijit-dev82 force-pushed the capability_fix branch 2 times, most recently from 12dee8c to 876c020 Compare July 27, 2022 05:09
@abhijit-dev82
Modified capabilties API to a generic call
bd99430 
Modified capabilties API to a generic call

Modified capabilties API to a generic call

Modified capabilties API to a generic call

Modified capabilties API to a generic call

Modified capabilties API to a generic call

Detect TKGS environment for any version of TKC API

Detect TKGS environment for any version of TKC API
@rajathagasthya rajathagasthya added the ok-to-merge PRs should be labelled with this before merging label Jul 27, 2022
@rajathagasthya rajathagasthya merged commit c73f092 into vmware-tanzu:main Jul 27, 2022
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Reviewers

@yharish991 yharish991 yharish991 approved these changes

@rajathagasthya rajathagasthya rajathagasthya approved these changes

Assignees
No one assigned
Labels
cla-not-required ok-to-merge PRs should be labelled with this before merging
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Account for TKC API version removal in capability discovery functions
4 participants
@abhijit-dev82 @rajathagasthya @yharish991 @vmwclabot