[litmus-portal]Updating to use kubernetes patch instead of get-update in subscriber for updating cm and secret #3981

achiverram28 · 2023-05-19T09:23:15Z

Proposed changes

The ClusterRegister function in the litmus/litmus-portal/cluster-agents/subscriber/pkg/k8s
/operations.go has been modified to use kubernetes patch instead of update in subscriber for updating cm and secret

This solves the issue #3944

I have made use of the json.Marshal for the clusterData and have used it in Patch(MergePatchType) for both configMap and Secret patch.

Types of changes

What types of changes does your code introduce to Litmus? Put an x in the boxes that apply

New feature (non-breaking change which adds functionality)
Bugfix (non-breaking change which fixes an issue)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation Update (if none of the other choices applies)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

I have read the CONTRIBUTING doc
I have signed the commit for DCO to be passed.
Lint and unit tests pass locally with my changes
I have added tests that prove my fix is effective or that my feature works (if appropriate)
I have added necessary documentation (if appropriate)

Signed-off-by: achiverram28 <ramsamarth21bcs24@iiitkottayam.ac.in>

imrajdas

Thanks for this PR. Gave few comments, PTAL

imrajdas · 2023-05-20T14:41:27Z

litmus-portal/cluster-agents/subscriber/pkg/k8s/operations.go

@@ -158,52 +160,42 @@ func IsClusterConfirmed() (bool, string, error) {

 // ClusterRegister function creates litmus-portal config map in the litmus namespace
 func ClusterRegister(clusterData map[string]string) (bool, error) {


We need to patch two things

IS_CLUSTER_CONFIRMED

ACCESS_KEY

so, the func params can be be the access_key only and is_cluster_confirmed is a boolean(no need to add it in params)

Suggested change

func ClusterRegister(clusterData map[string]string) (bool, error) {

func ClusterRegister(accessKey string) (bool, error) {

imrajdas · 2023-05-20T14:41:55Z

litmus-portal/cluster-agents/subscriber/pkg/k8s/operations.go

-		"SKIP_SSL_VERIFY":      clusterData["SKIP_SSL_VERIFY"],
-		"CUSTOM_TLS_CERT":      clusterData["CUSTOM_TLS_CERT"],
+	configMapPatch:= map[string]interface{}{
+		"data": clusterData,


only need to patch IS_CLUSTER_CONFIRMED as mentioned above

imrajdas · 2023-05-20T14:42:28Z

litmus-portal/cluster-agents/subscriber/pkg/k8s/operations.go

-		"ACCESS_KEY": clusterData["ACCESS_KEY"],
-		"CLUSTER_ID": clusterData["CLUSTER_ID"],
+	secretPatch:= map[string]interface{}{
+		"stringData":clusterData,


need to patch ACCESS_KEY only

imrajdas · 2023-05-20T14:43:03Z

litmus-portal/cluster-agents/subscriber/pkg/k8s/operations.go


-	logrus.Info(AgentSecretName + " has been updated")
+	logrus.Info(AgentSecretName+" has been updated")


Suggested change

logrus.Info(AgentSecretName+" has been updated")

logrus.Infof("%s has been patched", AgentSecretName)

imrajdas · 2023-05-20T14:43:25Z

litmus-portal/cluster-agents/subscriber/pkg/k8s/operations.go

 	if err != nil {
 		return false, err
 	}

-	logrus.Info(AgentConfigName + " has been updated")
+	logrus.Info(AgentConfigName+" has been updated")


Suggested change

logrus.Info(AgentConfigName+" has been updated")

logrus.Infof("%s has been patched", AgentConfigName)

Sure I will makes these changes and update the PR

imrajdas · 2023-05-20T14:44:20Z

cc: @gdsoumya @amityt for reviews

…iber/pkg/k8s/operations.go Signed-off-by: achiverram28 <ramsamarth21bcs24@iiitkottayam.ac.in>

…agents/subscriber/pkg/k8s/operations.go Signed-off-by: achiverram28 <ramsamarth21bcs24@iiitkottayam.ac.in>

achiverram28 · 2023-05-20T18:39:38Z

@imrajdas I have updated the code and updated the pr . Can the maintainers have a look at it

imrajdas · 2023-05-20T20:13:10Z

litmus-portal/cluster-agents/subscriber/pkg/k8s/operations.go

@@ -157,53 +159,48 @@ func IsClusterConfirmed() (bool, string, error) {
 }

 // ClusterRegister function creates litmus-portal config map in the litmus namespace
-func ClusterRegister(clusterData map[string]string) (bool, error) {
-	clientset, err := GetGenericK8sClient()
+func ClusterRegister(accessKey string) (bool, error) {


Check where ClusterRegister() is being called and update the params there

gdsoumya

Please take a look at how patch works and whats the expected inputs for it and test the code locally to make sure it works.

gdsoumya · 2023-05-21T04:11:48Z

litmus-portal/cluster-agents/subscriber/pkg/k8s/operations.go

+	isClusterConfirmed, _, err := IsClusterConfirmed()
+	if err != nil {
+		return false, err


No need to call it again, pass it as param as suggested by raj.

gdsoumya · 2023-05-21T04:17:41Z

litmus-portal/cluster-agents/subscriber/pkg/k8s/operations.go

-		},
-	}, metav1.UpdateOptions{})
+	configMapPatch:= map[string]bool {
+		"data": isClusterConfirmed,


Did you test if this works? You need to specify the key-value mapping for the patch, here you just put the bool value directly under data

@achiverram28 You can try something like this

updatedConfigMap := corev1.ConfigMap{ Data: map[string]string{ "<key>": "<value>", }, } configMapPatch, err := json.Marshal(updatedConfigMap) if err != nil { return fmt.Errorf("failed to marshal patched config, err: %v", err) }

gdsoumya · 2023-05-21T04:17:58Z

litmus-portal/cluster-agents/subscriber/pkg/k8s/operations.go

+	logrus.Info("%s has been patched",AgentConfigName)
+
+	secretPatch:= map[string]string{
+		"stringData": accessKey,


same here need to specify the map

amityt · 2023-05-21T17:09:07Z

litmus-portal/cluster-agents/subscriber/pkg/k8s/operations.go

-		"VERSION":              clusterData["VERSION"],
-		"SKIP_SSL_VERIFY":      clusterData["SKIP_SSL_VERIFY"],
-		"CUSTOM_TLS_CERT":      clusterData["CUSTOM_TLS_CERT"],
+	isClusterConfirmed, _, err := IsClusterConfirmed()


As mentioned by Shomo, this function is not needed here. Instead, you can directly set the value to true since we are already updating the map here:

litmus/litmus-portal/cluster-agents/subscriber/subscriber.go

Line 115 in cf86f7e

clusterData["IS_CLUSTER_CONFIRMED"] = "true"

amityt

Please make the requested changes @achiverram28

achiverram28 · 2023-05-21T18:00:55Z

Yes , I will work on all of these changes and update the pr . Thanks for the guidance

…s.go and litmus-portal/cluster-agents/subscriber/subscriber.go Signed-off-by: achiverram28 <ramsamarth21bcs24@iiitkottayam.ac.in>

achiverram28 · 2023-05-21T21:11:53Z

I have made changes by updating the params of the ClusterRegister() in subscriber.go @imrajdas .
I have removed the function call inside and studied the subscriber.go and passed it as a parameter to ClusterRegister() . So I have passed accessKey and isClusterConfirmed as arguments in subscriber.go @imrajdas @gdsoumya @amityt .
Sorry for the mistake during ConfigMap and Secret creation . As per the guidance of @amityt and @gdsoumya I properly studied this usage and input for those and modified by adding maps .
Hope this solves all the requested changes . If any more changes I am open for it.

Sir , what shall we do about the AgentNamespace since it was commented , in the original code , how should we deal with that . This one

Signed-off-by: achiverram28 <ramsamarth21bcs24@iiitkottayam.ac.in>

vanshBhatia-A4k9 · 2023-12-08T06:18:46Z

The litmus-portal directory is a legacy directory (will be depreciated soon), and the latest code base derives its logic from the chaos-centre directory, can you please make your changes there.
Example: make changes in chaoscenter/subscriber/pkg/k8s/operations.go instead of litmus-portal/cluster-agents/subscriber/pkg/k8s/operations.go. cc: @Saranya-jena

Updating the ClusterRegister function to use Patch from Update

b048878

Signed-off-by: achiverram28 <ramsamarth21bcs24@iiitkottayam.ac.in>

imrajdas requested changes May 20, 2023

View reviewed changes

imrajdas requested review from amityt and gdsoumya May 20, 2023 14:44

achiverram28 added 2 commits May 20, 2023 23:55

Modified the requested changes in litmus-portal/cluster-agents/subscr…

0757cb9

…iber/pkg/k8s/operations.go Signed-off-by: achiverram28 <ramsamarth21bcs24@iiitkottayam.ac.in>

Modified to follow more better code quality in litmus-portal/cluster-…

21acdae

…agents/subscriber/pkg/k8s/operations.go Signed-off-by: achiverram28 <ramsamarth21bcs24@iiitkottayam.ac.in>

imrajdas reviewed May 20, 2023

View reviewed changes

gdsoumya requested changes May 21, 2023

View reviewed changes

amityt requested changes May 21, 2023

View reviewed changes

amityt reviewed May 21, 2023

View reviewed changes

Updated the litmus-portal/cluster-agents/subscriber/pkg/k8s/operation…

c50fa3d

…s.go and litmus-portal/cluster-agents/subscriber/subscriber.go Signed-off-by: achiverram28 <ramsamarth21bcs24@iiitkottayam.ac.in>

achiverram28 requested review from imrajdas, gdsoumya and amityt May 21, 2023 21:04

Modified to avoid module naming conflict

9bd23ab

Signed-off-by: achiverram28 <ramsamarth21bcs24@iiitkottayam.ac.in>

RipulHandoo mentioned this pull request Nov 4, 2023

Use kubernetes patch instead of get-update in subscriber for updating cm and secret #3944

Open

imrajdas approved these changes Dec 14, 2023

View reviewed changes

amityt approved these changes Dec 15, 2023

View reviewed changes

Saranya-jena closed this Dec 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[litmus-portal]Updating to use kubernetes patch instead of get-update in subscriber for updating cm and secret #3981

[litmus-portal]Updating to use kubernetes patch instead of get-update in subscriber for updating cm and secret #3981

achiverram28 commented May 19, 2023 •

edited

Loading

imrajdas left a comment

imrajdas May 20, 2023

imrajdas May 20, 2023

imrajdas May 20, 2023

imrajdas May 20, 2023

imrajdas May 20, 2023

achiverram28 May 20, 2023 •

edited

Loading

imrajdas commented May 20, 2023

achiverram28 commented May 20, 2023

imrajdas May 20, 2023

gdsoumya left a comment

gdsoumya May 21, 2023

gdsoumya May 21, 2023

amityt May 21, 2023

gdsoumya May 21, 2023

amityt May 21, 2023

amityt left a comment

achiverram28 commented May 21, 2023

achiverram28 commented May 21, 2023 •

edited

Loading

vanshBhatia-A4k9 commented Dec 8, 2023 •

edited

Loading

		@@ -158,52 +160,42 @@ func IsClusterConfirmed() (bool, string, error) {

		// ClusterRegister function creates litmus-portal config map in the litmus namespace
		func ClusterRegister(clusterData map[string]string) (bool, error) {

	func ClusterRegister(clusterData map[string]string) (bool, error) {
	func ClusterRegister(accessKey string) (bool, error) {


		logrus.Info(AgentSecretName + " has been updated")
		logrus.Info(AgentSecretName+" has been updated")

	logrus.Info(AgentSecretName+" has been updated")
	logrus.Infof("%s has been patched", AgentSecretName)

	logrus.Info(AgentConfigName+" has been updated")
	logrus.Infof("%s has been patched", AgentConfigName)

[litmus-portal]Updating to use kubernetes patch instead of get-update in subscriber for updating cm and secret #3981

[litmus-portal]Updating to use kubernetes patch instead of get-update in subscriber for updating cm and secret #3981

Conversation

achiverram28 commented May 19, 2023 • edited Loading

Proposed changes

Types of changes

Checklist

imrajdas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

achiverram28 May 20, 2023 • edited Loading

Choose a reason for hiding this comment

imrajdas commented May 20, 2023

achiverram28 commented May 20, 2023

Choose a reason for hiding this comment

gdsoumya left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amityt left a comment

Choose a reason for hiding this comment

achiverram28 commented May 21, 2023

achiverram28 commented May 21, 2023 • edited Loading

vanshBhatia-A4k9 commented Dec 8, 2023 • edited Loading

achiverram28 commented May 19, 2023 •

edited

Loading

achiverram28 May 20, 2023 •

edited

Loading

achiverram28 commented May 21, 2023 •

edited

Loading

vanshBhatia-A4k9 commented Dec 8, 2023 •

edited

Loading