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

Implement changes for Upgrades to the inline config of packages #3191

Merged

Conversation

shivaani0505
Copy link
Contributor

@shivaani0505 shivaani0505 commented Aug 24, 2022

What this PR does / why we need it

Implement changes for Upgrades to the inline config of packages

Which issue(s) this PR fixes

Fixes #3190

Describe testing done for PR

Release note

Upgrades to the inline config of packages should get reflected

Additional information

Special notes for your reviewer

Testing:
env tests pass for the changes

@codecov
Copy link

codecov bot commented Aug 24, 2022

Codecov Report

Merging #3191 (f08cb32) into main (9057a8e) will increase coverage by 0.69%.
The diff coverage is 63.26%.

@@            Coverage Diff             @@
##             main    #3191      +/-   ##
==========================================
+ Coverage   45.78%   46.48%   +0.69%     
==========================================
  Files         361      342      -19     
  Lines       37653    35298    -2355     
==========================================
- Hits        17240    16408     -832     
+ Misses      18836    17406    -1430     
+ Partials     1577     1484      -93     
Impacted Files Coverage Δ
addons/controllers/clusterbootstrap_controller.go 64.55% <51.35%> (+1.27%) ⬆️
...til/clusterbootstrapclone/clusterbootstrapclone.go 70.37% <100.00%> (+0.26%) ⬆️
...ntroller/tkr-status/clusterstatus/clusterstatus.go 45.35% <0.00%> (-2.19%) ⬇️
pkg/v1/tkg/kind/logger.go
pkg/v1/tkg/aws/profiles.go
pkg/v1/tkg/kind/utils.go
pkg/v1/tkg/region/factory.go
pkg/v1/tkg/utils/lock.go
pkg/v1/tkg/utils/kubeconfig.go
pkg/v1/tkg/utils/converter.go
... and 13 more

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

@vijaykatam vijaykatam self-assigned this Aug 25, 2022
Copy link
Contributor

@vijaykatam vijaykatam left a comment

Choose a reason for hiding this comment

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

Lets add tests for updating inline secret to clusterbootstrap_controller_test with 2 scenarios.

  1. Adding an inline value for an existing cb object.
  2. Updating an inline value for an existing cb object.

addons/controllers/clusterbootstrap_controller.go Outdated Show resolved Hide resolved
addons/controllers/clusterbootstrap_controller.go Outdated Show resolved Hide resolved
Copy link
Contributor

@vijaykatam vijaykatam left a comment

Choose a reason for hiding this comment

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

Change looks good to me. Please add tests as requested in #3191 (review) so that this change can be merged.

@shivaani0505 shivaani0505 force-pushed the shivaani/update-inline-config branch from ae1a737 to 76f63c0 Compare September 7, 2022 18:19
@shivaani0505 shivaani0505 requested a review from a team as a code owner September 8, 2022 00:45
@github-actions
Copy link

github-actions bot commented Sep 8, 2022

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3191/20220908005403/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
Copy link
Contributor Author

Change looks good to me. Please add tests as requested in #3191 (review) so that this change can be merged.

Test case has been added and I have verified with tkgs cluster
Test Logs:

Workload cluster 'cc-inline' created

STEP: Generating credentials for workload cluster "cc-inline"
Retrieving credentials for workload cluster cc-inline 
getting secret for cluster
waiting for resource cc-inline-kubeconfig of type *v1.Secret to be up and running
Merging credentials into kubeconfig file /Users/gshivaani/kubectl-vsphere-plugin/bin/testinline.config 
Credentials of cluster 'cc-inline' have been saved 
STEP: Get k8s client for workload cluster "cc-inline"
You can now access the cluster by running 'kubectl config use-context cc-inline-admin@cc-inline' under path '/var/folders/nd/q7bzmcrj5g70mdg92q3c7pb00000gp/T/cc-inline.kubeconfig' 
I0907 17:41:52.826848   95213 request.go:665] Waited for 1.030994962s due to client-side throttling, not priority and fairness, request: GET:https://192.168.116.2:6443/apis/stats.antrea.io/v1alpha1?timeout=32s
STEP: Update inline config for attribute periodSeconds for metrics server package for "cc-inline"

• [SLOW TEST:371.734 seconds]
TKGS ClusterClass based workload cluster tests
/Users/gshivaani/tanzu-framework/pkg/v1/tkg/test/tkgctl/tkgs_cc/tkgs_cc_workload_cluster_test.go:36
  when input file is cluster class based with custom Cluster Bootstrap
  /Users/gshivaani/tanzu-framework/pkg/v1/tkg/test/tkgctl/tkgs_cc/tkgs_cc_workload_cluster_test.go:118
    should create the data value secret in supervisor and guest cluster for a package with inline config
    /Users/gshivaani/tanzu-framework/pkg/v1/tkg/test/tkgctl/tkgs_cc/tkgs_cc_workload_cluster_test.go:181
------------------------------
SS
JUnit report was created: /var/folders/nd/q7bzmcrj5g70mdg92q3c7pb00000gp/T/artifacts/junit/junit.e2e_suite.1.xml

Ran 1 of 10 Specs in 372.798 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 9 Skipped
PASS | FOCUSED

Copy link
Contributor

@vijaykatam vijaykatam 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. Thanks for updating tests!

@vijaykatam vijaykatam added the ok-to-merge PRs should be labelled with this before merging label Sep 8, 2022
@shivaani0505 shivaani0505 merged commit 8ada649 into vmware-tanzu:main Sep 8, 2022
# 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.

Upgrades to the inline config of packages should get reflected
4 participants