fix: check nodeclass before controller start

[cvvz]

Reason for Change:

Check nodeclass before controller start, rather than check before creating nodeclaim.
In this way there is no need to add a nodeclass informer #1069

Requirements

• added unit tests and e2e tests (if applicable).

Issue Fixed:

Notes for Reviewers:

[kaito-pr-agent]

Title

(Describe updated until commit 7f41dbe)

Enhance nodeclass check and validate CLOUD_PROVIDER

---

Description

• Added validation for CLOUD_PROVIDER environment variable

• Moved nodeclass check to before controller start

• Removed redundant nodeclass check during NodeClaim creation

---

Changes walkthrough 📝

	Relevant files
Enhancement	main.go Validate CLOUD_PROVIDER and move nodeclass check                  cmd/ragengine/main.go Imported additional packages for feature gates and constants Added validation for CLOUD_PROVIDER environment variable Moved nodeclass check to before controller start +15/-0    main.go Validate CLOUD_PROVIDER and move nodeclass check                  cmd/workspace/main.go Imported additional packages for feature gates and constants Added validation for CLOUD_PROVIDER environment variable Moved nodeclass check to before controller start +14/-0    common.go Add cloud provider validation function                                      pkg/utils/common.go Added function ValidCloudProvider to validate cloud provider names +9/-0
Refactoring	nodeclaim.go Remove redundant nodeclass check                                                  pkg/utils/nodeclaim/nodeclaim.go Removed redundant nodeclass check during NodeClaim creation +0/-8

---

Need help?

Type /help how to ... in the comments thread for any questions about PR-Agent usage.

Check out the documentation for more information.

[kaito-pr-agent]

PR Reviewer Guide 🔍

(Review updated until commit 7f41dbe)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Feature Gate Usage The feature gate FeatureFlagEnsureNodeClass is checked before calling CheckNodeClass. Ensure that this feature gate is properly documented and tested. if featuregates.FeatureGates[consts.FeatureFlagEnsureNodeClass] { err := nodeclaim.CheckNodeClass(ctx, kClient) if err != nil { exitWithErrorFunc() } } Feature Gate Usage The feature gate FeatureFlagEnsureNodeClass is checked before calling CheckNodeClass. Ensure that this feature gate is properly documented and tested. if featuregates.FeatureGates[consts.FeatureFlagEnsureNodeClass] { err := nodeclaim.CheckNodeClass(ctx, kClient) if err != nil { exitWithErrorFunc() } } Cloud Provider Validation The function ValidCloudProvider checks against a set of constants. Ensure that these constants are comprehensive and up-to-date with all supported cloud providers. func ValidCloudProvider(cloudProvider string) bool { switch cloudProvider { case consts.AzureCloudName, consts.AWSCloudName, consts.ArcCloudName: return true default: return false }

[kaito-pr-agent]

PR Code Suggestions ✨

Latest suggestions up to 7f41dbe
Explore these optional code suggestions:

Category	Suggestion	Impact
General	Avoid redundant node class checks Ensure CheckNodeClass is called only once per process to avoid redundant checks. cmd/ragengine/main.go [123-127] if featuregates.FeatureGates[consts.FeatureFlagEnsureNodeClass] { err := nodeclaim.CheckNodeClass(ctx, kClient) if err != nil { + klog.ErrorS(err, "failed to check node class") exitWithErrorFunc() } } Suggestion importance[1-10]: 5 __ Why: The suggestion aims to improve the code by adding a more descriptive error message, which enhances readability and debugging. However, it does not address redundancy since the check is already conditional based on a feature gate.	Low

---

Previous suggestions

Suggestions up to commit 7f41dbe

Category	Suggestion	Impact
General	Avoid redundant node class checks Ensure CheckNodeClass is called only once per process to avoid redundant checks. cmd/ragengine/main.go [123-127] if featuregates.FeatureGates[consts.FeatureFlagEnsureNodeClass] { err := nodeclaim.CheckNodeClass(ctx, kClient) if err != nil { + klog.ErrorS(err, "failed to check node class") exitWithErrorFunc() } } Suggestion importance[1-10]: 5 __ Why: The suggestion aims to prevent redundant checks, but it doesn't address the redundancy issue since the check is already conditionally executed based on a feature gate. The improvement in logging is minor and doesn't significantly impact the functionality.	Low
	Extend cloud provider validation Consider adding validation for additional cloud providers if they are supported. pkg/utils/common.go [318-324] func ValidCloudProvider(cloudProvider string) bool { switch cloudProvider { case consts.AzureCloudName, consts.AWSCloudName, consts.ArcCloudName: return true + // Add additional cases for other cloud providers if needed default: return false } } Suggestion importance[1-10]: 3 __ Why: The suggestion is about extending validation for additional cloud providers, which is a good practice. However, it's more of a placeholder comment rather than an actionable change, hence the lower score.	Low

[Fei-Guo]

The old code was trying to avoid a case where user deletes the nodeclass accidentally. Since we are not adding nodeclass watcher, if we don't check nodeclass every time before creating a node and user deletes the nodeclass, the only remediation is to ask the user to delete the controller manually to force it to restart.

So ideally, we may need to add a separate controller for nodeclass only to strictly recover it whenever it is deleted. My thought right now is that the churn rate of creating nodeclaim by workspace controller is low. Hence, the overhead of performing a nodeclass check before creating every nodeclaim is low. I would suggest leaving current behavior unchanged. It is fair to add a TODO to optimize this behavior later.