Printing overrided vars in cli command `noobaa diagnostics report` #1445

achouhan09 · 2024-09-25T07:17:14Z

Explain the changes

Printing overriden vars in cli command noobaa diagnostics report.

Output:

Testing Instructions:

After installing noobaa try running noobaa diagnostics report cli command to check the status.

shirady

LGTM

pkg/diagnostics/report.go

shirady · 2024-09-25T09:25:58Z

@achouhan09 Could you please update the testing instructions in the PR?

Neon-White · 2024-09-25T09:40:16Z

I was just wondering - is there a reason we're only checking the core pod for overrides? I think it'd be very useful and important to check one of the endpoints as well

achouhan09 · 2024-09-25T10:49:48Z

I was just wondering - is there a reason we're only checking the core pod for overrides? I think it'd be very useful and important to check one of the endpoints as well

@Neon-White Yes, make sense. I have added a check for endpoint as well. Thanks

pkg/diagnostics/report.go

liranmauda · 2024-09-25T11:31:31Z

I was just wondering - is there a reason we're only checking the core pod for overrides? I think it'd be very useful and important to check one of the endpoints as well

I was just wondering - is there a reason we're only checking the core pod for overrides? I think it'd be very useful and important to check one of the endpoints as well

@Neon-White Yes, make sense. I have added a check for endpoint as well. Thanks

@Neon-White @achouhan09 Do we also need to add the proxy check on the endpoint?

achouhan09 · 2024-09-25T11:45:39Z

I was just wondering - is there a reason we're only checking the core pod for overrides? I think it'd be very useful and important to check one of the endpoints as well

I was just wondering - is there a reason we're only checking the core pod for overrides? I think it'd be very useful and important to check one of the endpoints as well

@Neon-White Yes, make sense. I have added a check for endpoint as well. Thanks

@Neon-White @achouhan09 Do we also need to add the proxy check on the endpoint?

Not sure, but for proxy changes I need to update the operator deployment and then it will get reflected to other pods, so for proxy status we can check for any one of the core/endpoint is updated or not. @Neon-White pls correct me if I am wrong.

Neon-White · 2024-09-26T09:01:24Z

@achouhan09 @liranmauda
"After configuring the cluster-wide proxy, the Operator Lifecycle Manager (OLM) triggers automatic updates to all of the deployed Operators with the new contents of the HTTP_PROXY, HTTPS_PROXY, and NO_PROXY environment variables."

Ideally, this is how users would configure their proxies, which would be a reliable way to detect it.
Depending on how proactive we want to be, it can still be valuable to check the core and endpoint individually, on top of the operator, just in case the customer applied a proxy on them in particular.

Signed-off-by: Aayush Chouhan <[email protected]>

liranmauda · 2024-09-29T10:27:47Z

@achouhan09 @liranmauda "After configuring the cluster-wide proxy, the Operator Lifecycle Manager (OLM) triggers automatic updates to all of the deployed Operators with the new contents of the HTTP_PROXY, HTTPS_PROXY, and NO_PROXY environment variables."

Ideally, this is how users would configure their proxies, which would be a reliable way to detect it. Depending on how proactive we want to be, it can still be valuable to check the core and endpoint individually, on top of the operator, just in case the customer applied a proxy on them in particular.

Great!
@achouhan09 lets merge this PR, and then create a new PR for the proxy to check on both core, and operator

Signed-off-by: Aayush Chouhan <[email protected]>

pkg/diagnostics/report.go

liranmauda · 2024-09-30T06:45:35Z

pkg/diagnostics/report.go

+	fmt.Print("Overridden Environment Variables Check (NOOBAA-CORE):\n----------------------------------\n")
+	foundCoreEnv := false
+	for _, envVar := range coreApp.Spec.Template.Spec.Containers[0].Env {
+		if strings.HasPrefix(envVar.Name, "CONFIG_JS_") {
+			fmt.Printf("    	✔ %s : %s\n", envVar.Name, envVar.Value)
+			foundCoreEnv = true
+		}
+	}
+	if !foundCoreEnv {
+		fmt.Printf("	❌ No overridden environment variables found.")
+	}
+	fmt.Println("")
+
+	fmt.Print("Overridden Environment Variables Check (ENDPOINT):\n----------------------------------\n")
+	foundEndpointEnv := false
+	for _, envVar := range endpointApp.Spec.Template.Spec.Containers[0].Env {
+		if strings.HasPrefix(envVar.Name, "CONFIG_JS_") {
+			fmt.Printf("    	✔ %s : %s\n", envVar.Name, envVar.Value)
+			foundEndpointEnv = true
+		}
+	}
+	if !foundEndpointEnv {
+		fmt.Printf("	❌ No overridden environment variables found.")
+	}


This duplication can be a function that gets a kubernetes object and is called twice, once for core sts and a second time for endpoint deployment.
@achouhan09 Create a new PR that consolidates those into a single function.

liranmauda · 2024-09-30T06:45:59Z

pkg/diagnostics/report.go

+	coreApp := util.KubeObject(bundle.File_deploy_internal_statefulset_core_yaml).(*appsv1.StatefulSet)
+	coreApp.Namespace = options.Namespace
+	if !util.KubeCheck(coreApp) {
+		log.Fatalf(`❌ Could not get core StatefulSet %q in Namespace %q`,
+			coreApp.Name, coreApp.Namespace)
+	}
+
+	// Fetching endpoint configurations
+	endpointApp := util.KubeObject(bundle.File_deploy_internal_deployment_endpoint_yaml).(*appsv1.Deployment)
+	endpointApp.Namespace = options.Namespace
+	if !util.KubeCheck(endpointApp) {
+		log.Fatalf(`❌ Could not get endpoint Deployment %q in Namespace %q`,
+			endpointApp.Name, endpointApp.Namespace)
+	}


This duplication can be a function that gets a kubernetes object and is called twice, once for core sts and a second time for endpoint deployment.
@achouhan09 Create a new PR that consolidates those into a single function.

achouhan09 requested review from Neon-White, liranmauda, shirady, a team and vh05 and removed request for a team September 25, 2024 07:17

pull-request-size bot added the size/M label Sep 25, 2024

shirady approved these changes Sep 25, 2024

View reviewed changes

pkg/diagnostics/report.go Outdated Show resolved Hide resolved

Neon-White approved these changes Sep 25, 2024

View reviewed changes

achouhan09 requested a review from Neon-White September 25, 2024 10:51

liranmauda reviewed Sep 25, 2024

View reviewed changes

pkg/diagnostics/report.go Outdated Show resolved Hide resolved

achouhan09 force-pushed the config_js_var branch from 080d4f2 to a834825 Compare September 25, 2024 11:15

achouhan09 requested a review from liranmauda September 25, 2024 11:17

liranmauda reviewed Sep 25, 2024

View reviewed changes

pkg/diagnostics/report.go Outdated Show resolved Hide resolved

pkg/diagnostics/report.go Outdated Show resolved Hide resolved

pkg/diagnostics/report.go Outdated Show resolved Hide resolved

pkg/diagnostics/report.go Outdated Show resolved Hide resolved

Printing overrided vars in cli command noobaa diagnostics report

46b5d57

Signed-off-by: Aayush Chouhan <[email protected]>

achouhan09 force-pushed the config_js_var branch from a834825 to 266508b Compare September 30, 2024 05:43

Added a check for endpoint and some fixes

6eaff91

Signed-off-by: Aayush Chouhan <[email protected]>

achouhan09 force-pushed the config_js_var branch from 266508b to 6eaff91 Compare September 30, 2024 05:45

achouhan09 merged commit 7611fca into noobaa:master Sep 30, 2024
14 checks passed

achouhan09 deleted the config_js_var branch September 30, 2024 06:25

liranmauda reviewed Sep 30, 2024

View reviewed changes

achouhan09 mentioned this pull request Oct 3, 2024

Added proxy check for endpoint and removed duplicate code #1455

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Printing overrided vars in cli command `noobaa diagnostics report` #1445

Printing overrided vars in cli command `noobaa diagnostics report` #1445

achouhan09 commented Sep 25, 2024 •

edited

Loading

shirady left a comment

shirady commented Sep 25, 2024

Neon-White commented Sep 25, 2024

achouhan09 commented Sep 25, 2024

liranmauda commented Sep 25, 2024

achouhan09 commented Sep 25, 2024

Neon-White commented Sep 26, 2024

liranmauda commented Sep 29, 2024

liranmauda Sep 30, 2024

liranmauda Sep 30, 2024

Printing overrided vars in cli command noobaa diagnostics report #1445

Printing overrided vars in cli command noobaa diagnostics report #1445

Conversation

achouhan09 commented Sep 25, 2024 • edited Loading

Explain the changes

Testing Instructions:

shirady left a comment

Choose a reason for hiding this comment

shirady commented Sep 25, 2024

Neon-White commented Sep 25, 2024

achouhan09 commented Sep 25, 2024

liranmauda commented Sep 25, 2024

achouhan09 commented Sep 25, 2024

Neon-White commented Sep 26, 2024

liranmauda commented Sep 29, 2024

liranmauda Sep 30, 2024

Choose a reason for hiding this comment

liranmauda Sep 30, 2024

Choose a reason for hiding this comment

Printing overrided vars in cli command `noobaa diagnostics report` #1445

Printing overrided vars in cli command `noobaa diagnostics report` #1445

achouhan09 commented Sep 25, 2024 •

edited

Loading