⚠️ Mark singleton flag in init deprecated#532
⚠️ Mark singleton flag in init deprecated#532qiujian16 wants to merge 1 commit intoopen-cluster-management-io:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: qiujian16 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
WalkthroughThis PR deprecates two init command flags ("singleton" and "singleton-name") directing users to Helm chart installation, and removes the MulticlusterControlplane field from the VersionBundle struct across all version entries and related tests. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@pkg/cmd/init/cmd.go`:
- Around line 63-64: Move the call to MarkDeprecated so it runs after the flag
is registered: define the flag with cmd.Flags().BoolVar(&o.singleton,
"singleton", false, ...) first (the BoolVar call that sets up o.singleton), then
call cmd.Flags().MarkDeprecated("singleton", "install via helm chart directly")
to ensure the FlagSet contains "singleton" before marking it deprecated; update
any error handling around MarkDeprecated instead of discarding returned errors.
- Around line 88-89: The deprecated call is invoked before the flag is defined
and is using the wrong FlagSet; move the MarkDeprecated call to after the flag
is created and call it on the same FlagSet that defines the flag (i.e., after
singletonSet.StringVar(...)) and use
singletonSet.MarkDeprecated("singleton-name", "install via helm chart directly")
so the flag exists and the deprecation is applied on the correct FlagSet.
In `@pkg/version/version_test.go`:
- Around line 10-13: Tests fail because they request a non-existent bundle
"v0.16.0"; update the tests or bundles so the lookup in getVersionBundle
succeeds: either change the test cases that use "v0.16.0" (e.g., the "specific
version" and "override" tests and the expectedVersionBundle variable) to an
existing key such as "v1.0.0"/"1.0.0" or "latest"/"default", or add a
corresponding entry for "0.16.0" into the versionBundleList used by
getVersionBundle in version.go so that VersionBundle lookups succeed and the
override test can apply the bundle-overrides.json as intended.
pkg/cmd/init/cmd.go
Outdated
There was a problem hiding this comment.
Same issue: MarkDeprecated called before singleton-name flag is defined.
Similar to the singleton flag, MarkDeprecated("singleton-name", ...) is called on line 88 before the flag is defined on line 89. The flag must exist before it can be marked as deprecated.
🐛 Proposed fix
singletonSet := pflag.NewFlagSet("singletonSet", pflag.ExitOnError)
- _ = cmd.Flags().MarkDeprecated("singleton-name", "install via helm chart directly")
singletonSet.StringVar(&o.SingletonName, "singleton-name", "singleton-controlplane", "The name of the singleton control plane")
+ _ = singletonSet.MarkDeprecated("singleton-name", "install via helm chart directly")
_ = clusterManagerSet.SetAnnotation("singleton-name", "singletonSet", []string{})Note: The flag is defined on singletonSet, not cmd.Flags(), so MarkDeprecated should also be called on singletonSet.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| _ = cmd.Flags().MarkDeprecated("singleton-name", "install via helm chart directly") | |
| singletonSet.StringVar(&o.SingletonName, "singleton-name", "singleton-controlplane", "The name of the singleton control plane") | |
| singletonSet := pflag.NewFlagSet("singletonSet", pflag.ExitOnError) | |
| singletonSet.StringVar(&o.SingletonName, "singleton-name", "singleton-controlplane", "The name of the singleton control plane") | |
| _ = singletonSet.MarkDeprecated("singleton-name", "install via helm chart directly") | |
| _ = clusterManagerSet.SetAnnotation("singleton-name", "singletonSet", []string{}) |
🤖 Prompt for AI Agents
In `@pkg/cmd/init/cmd.go` around lines 88 - 89, The deprecated call is invoked
before the flag is defined and is using the wrong FlagSet; move the
MarkDeprecated call to after the flag is created and call it on the same FlagSet
that defines the flag (i.e., after singletonSet.StringVar(...)) and use
singletonSet.MarkDeprecated("singleton-name", "install via helm chart directly")
so the flag exists and the deprecation is applied on the correct FlagSet.
qiujian16
force-pushed
the
mark-controleplane-singleton-deprecated
branch
from
7c4bdc6 to
d73b1b1
Compare
Signed-off-by: Jian Qiu <jqiu@redhat.com>
qiujian16
force-pushed
the
mark-controleplane-singleton-deprecated
branch
from
d73b1b1 to
b11e0dd
Compare
|
/lgtm |
3 participants
Summary
Related issue(s)
Fixes #
Summary by CodeRabbit
Deprecations
Changes