✨ Completing aws registration on spoke #788

jaswalkiranavtar · 2025-01-06T22:33:05Z

Summary

This PR has enhancement to complete the registration on spoke and save hub kubeconfig in a secret on spoke

Related issue(s)

Fixes #514

Signed-off-by: suvaanshkumar <[email protected]>

codecov · 2025-01-07T22:20:41Z

Codecov Report

Attention: Patch coverage is 53.06122% with 23 lines in your changes missing coverage. Please review.

Project coverage is 63.80%. Comparing base (037aa3c) to head (cb0df69).
Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/registration/register/aws_irsa/aws.go	0.00%	12 Missing ⚠️
pkg/registration/spoke/spokeagent.go	16.66%	4 Missing and 1 partial ⚠️
...lers/klusterletcontroller/klusterlet_controller.go	57.14%	3 Missing ⚠️
pkg/registration/register/aws_irsa/aws_irsa.go	91.30%	2 Missing ⚠️
pkg/registration/register/aws_irsa/options.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #788      +/-   ##
==========================================
+ Coverage   63.78%   63.80%   +0.02%     
==========================================
  Files         192      192              
  Lines       18606    18642      +36     
==========================================
+ Hits        11868    11895      +27     
- Misses       5759     5770      +11     
+ Partials      979      977       -2

Flag	Coverage Δ
unit	`63.80% <53.06%> (+0.02%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

qiujian16 · 2025-01-08T01:28:34Z

need to run make fmt-imports

qiujian16 · 2025-01-08T01:29:55Z

pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go

+
+func GetAwsRegion(clusterArn string) string {
+	clusterStringParts := strings.Split(clusterArn, ":")
+	return clusterStringParts[3]


do we ensure that the slice size is 4?

The hub cluster arn shoudl be of the format arn:aws:eks:::cluster/ so the length should be fine.

qiujian16 · 2025-01-08T01:30:44Z

pkg/registration/register/aws_irsa/aws.go

 	approved := false
-
+	for _, condition := range v1Managedcluster.Status.Conditions {


you could use meta.FindStatusCondition here.

qiujian16 · 2025-01-08T01:35:32Z

test/integration/util/authentication.go

+	return secret, nil
+}
+
+func GetFilledAWSHubKubeConfigSecret(kubeClient kubernetes.Interface, secretNamespace, secretName string) (*corev1.Secret, error) {


if it is only to check the hubkubeconfig key, you might want to call it GetHubKubeConfigFromSecret

qiujian16 · 2025-01-08T01:37:32Z

test/integration/registration/spokecluster_aws_joining_test.go

-			//	}
-			//	return nil
-			//}, eventuallyTimeout, eventuallyInterval).ShouldNot(gomega.HaveOccurred())
+			// ensure that generated hub-kubeconfig-secret is correct


let's also comment that the kubeconfig secret here in integration test for AWS won't be able to connect to hub server, since it is not in the eks environment. So we only validate the format of the hub-kubeconfig.

Signed-off-by: suvaanshkumar <[email protected]>

qiujian16 · 2025-01-09T01:15:11Z

test/integration/registration/spokecluster_aws_joining_test.go

@@ -117,3 +168,12 @@ var _ = ginkgo.Describe("Joining Process for aws flow", func() {
 	})

 })
+
+func contains(slice []string, value string) bool {


nit: alternatively, you could use

stringSet := sets.New[string](slice...) return stringSet.Has(value) ....

qiujian16 · 2025-01-09T11:13:41Z

/approve

only 1 minor comment, feel free to tag when it is resolved.

jaswalkiranavtar · 2025-01-09T18:18:07Z

@mikeshng We have addressed qiujian's final comment. Could you please help merge it.

Signed-off-by: Gaurav Jaswal <[email protected]>

mikeshng

/lgtm

Confirmed the sets lib comment from reviewer has been addressed.

openshift-ci · 2025-01-09T18:31:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jaswalkiranavtar, mikeshng, qiujian16

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~pkg/operator/OWNERS~~ [qiujian16]
~~pkg/registration/OWNERS~~ [qiujian16]
~~test/OWNERS~~ [qiujian16]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci bot requested review from dongbeiqing91 and elgnay January 6, 2025 22:33

suvaanshkumar force-pushed the spoke-agent-registration-completion branch 12 times, most recently from b1cffe9 to 5e288e7 Compare January 7, 2025 20:50

Completing aws registration on spoke

8fa0860

Signed-off-by: suvaanshkumar <[email protected]>

suvaanshkumar force-pushed the spoke-agent-registration-completion branch 2 times, most recently from c755023 to 1b1d1da Compare January 7, 2025 22:15

suvaanshkumar force-pushed the spoke-agent-registration-completion branch from 1b1d1da to 387a974 Compare January 7, 2025 22:48

qiujian16 reviewed Jan 8, 2025

View reviewed changes

suvaanshkumar force-pushed the spoke-agent-registration-completion branch 6 times, most recently from 3912c02 to 875ec73 Compare January 8, 2025 22:18

adding new function to overcome gci errors on slices

22a64b5

Signed-off-by: suvaanshkumar <[email protected]>

suvaanshkumar force-pushed the spoke-agent-registration-completion branch from 875ec73 to 22a64b5 Compare January 8, 2025 22:19

qiujian16 reviewed Jan 9, 2025

View reviewed changes

openshift-ci bot added the approved label Jan 9, 2025

Refactoring array contains function

cb0df69

Signed-off-by: Gaurav Jaswal <[email protected]>

jaswalkiranavtar force-pushed the spoke-agent-registration-completion branch from 49ebc19 to cb0df69 Compare January 9, 2025 18:24

mikeshng approved these changes Jan 9, 2025

View reviewed changes

openshift-ci bot assigned mikeshng Jan 9, 2025

openshift-ci bot added the lgtm label Jan 9, 2025

openshift-merge-bot bot merged commit 0acf030 into open-cluster-management-io:main Jan 9, 2025
14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Completing aws registration on spoke #788

✨ Completing aws registration on spoke #788

jaswalkiranavtar commented Jan 6, 2025 •

edited

Loading

codecov bot commented Jan 7, 2025 •

edited

Loading

qiujian16 commented Jan 8, 2025

qiujian16 Jan 8, 2025

suvaanshkumar Jan 8, 2025

qiujian16 Jan 8, 2025

qiujian16 Jan 8, 2025

qiujian16 Jan 8, 2025

qiujian16 Jan 9, 2025 •

edited

Loading

qiujian16 commented Jan 9, 2025

jaswalkiranavtar commented Jan 9, 2025

mikeshng left a comment

openshift-ci bot commented Jan 9, 2025

		approved := false

		for _, condition := range v1Managedcluster.Status.Conditions {

✨ Completing aws registration on spoke #788

✨ Completing aws registration on spoke #788

Conversation

jaswalkiranavtar commented Jan 6, 2025 • edited Loading

Summary

Related issue(s)

codecov bot commented Jan 7, 2025 • edited Loading

Codecov Report

qiujian16 commented Jan 8, 2025

qiujian16 Jan 8, 2025

Choose a reason for hiding this comment

suvaanshkumar Jan 8, 2025

Choose a reason for hiding this comment

qiujian16 Jan 8, 2025

Choose a reason for hiding this comment

qiujian16 Jan 8, 2025

Choose a reason for hiding this comment

qiujian16 Jan 8, 2025

Choose a reason for hiding this comment

qiujian16 Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

qiujian16 commented Jan 9, 2025

jaswalkiranavtar commented Jan 9, 2025

mikeshng left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Jan 9, 2025

jaswalkiranavtar commented Jan 6, 2025 •

edited

Loading

codecov bot commented Jan 7, 2025 •

edited

Loading

qiujian16 Jan 9, 2025 •

edited

Loading