kubeadm: Remove the usage to print the default component configs for `reset` and `join` #119346

chendave · 2023-07-15T11:24:41Z

component configs is only needed for kubeadm init, the join and reset doesn't need to provid the config with component configs.

/kind bug

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

kubeadm: print the default component configs for `reset` and `join` is now not supported

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot · 2023-07-15T11:24:49Z

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

SataQiu · 2023-07-16T03:29:10Z

Even it's possible by the CLI, but it shouldn't be guaranteed by any testcases.

I'm wondering if we should disable the --component-configs flag for reset.
What do you think? @neolit123 @pacoxu

neolit123 · 2023-07-16T05:23:40Z

Even it's possible by the CLI, but it shouldn't be guaranteed by any testcases.

I'm wondering if we should disable the --component-configs flag for reset. What do you think? @neolit123 @pacoxu

print for join also doesn’t need it?
if we already have it for join, it needs deprecation.

instead...we can keep it enabled everywhere but a No-op for reset, join. i might be missing some context.

SataQiu · 2023-07-16T08:01:47Z

we can keep it enabled everywhere but a No-op for reset, join.

Yes, these default configurations are only printed when the --component-configs flag is specified, it seems harmless.

neolit123

/approve
seems ok to clean these

pacoxu · 2023-07-17T08:03:59Z

print for join also doesn’t need it? if we already have it for join, it needs deprecation.

I prefer to deprecate it as it is not needed(harmless as well).

instead...we can keep it enabled everywhere but a No-op for reset, join. i might be missing some context.

It will make users confused like the below.

kubeadm config print reset-defaults --component-configs=KubeProxyConfiguration

I'm wondering if we should disable the --component-configs flag for reset.

+1

However, all can be a follow-up. This PR lgtm.

chendave · 2023-07-17T09:05:25Z

/hold

this needs to be polished, we can do more instead of just remove those testcases.

chendave · 2023-08-07T04:12:03Z

/retitle kubeadm: Deprecate the default values of component config for reset and join

/unhold

chendave · 2023-08-07T05:32:13Z

/test pull-kubernetes-e2e-gce

pacoxu · 2023-08-28T02:14:33Z

cmd/kubeadm/app/cmd/config.go

@@ -115,6 +115,7 @@ func newCmdConfigPrintResetDefaults(out io.Writer) *cobra.Command {

 func newCmdConfigPrintActionDefaults(out io.Writer, action string, configBytesProc func() ([]byte, error)) *cobra.Command {
 	kinds := []string{}
+	deprecatedMsg := fmt.Sprintf("Print the default value of component config API objects is deprecated for action: %s, it will be ingored if set and will be removed in a future release.", action)


typo: ignore.

a newline at the end? Or the first line of the configuration will be print just after this line.

Done.

print it to stdout use fmt.Fprintln instead.

dims · 2023-08-29T16:39:41Z

/assign @neolit123

neolit123 · 2023-08-29T18:21:12Z

cmd/kubeadm/app/cmd/config.go

@@ -115,6 +115,7 @@ func newCmdConfigPrintResetDefaults(out io.Writer) *cobra.Command {

 func newCmdConfigPrintActionDefaults(out io.Writer, action string, configBytesProc func() ([]byte, error)) *cobra.Command {
 	kinds := []string{}
+	deprecatedMsg := fmt.Sprintf("Print the default value of component config API objects is deprecated for action: %s, it will be ignored if set and will be removed in a future release.", action)


i may have misunderstood something around our discussion for UpgradeConfiguration?
we should continue printing the kubeadm's understanding of default component config, but we can stop managing component config via --config on upgrade.

EDIT: never mind. i'm losing context on what these changes are about.

neolit123

can we go without deprecation here and just remove the usage of component-configs for anything but init?

it feels like a bug to allow join, reset...etc to use these.

chendave · 2023-08-30T02:15:26Z

can we go without deprecation here and just remove the usage of component-configs for anything but init?

this is following the suggestion from @pacoxu , your input here? are you okay if we go without deprecation?

From @pacoxu

I prefer to deprecate it as it is not needed(harmless as well).

chendave · 2023-08-30T02:17:23Z

also, this needs a release note, will add it in the next iteration.

pacoxu · 2023-08-30T03:44:16Z

/kind bug

it feels like a bug to allow join, reset...etc to use these.

Just remove it if this is a bug 😄.

chendave · 2023-08-30T07:11:06Z

can we go without deprecation here and just remove the usage of component-configs for anything but init?

Done

chendave · 2023-08-30T07:11:47Z

/kind bug

neolit123

/approve
SGTM

please always prefix kubeadm release notes with kubeadm:

k8s-ci-robot · 2023-08-30T07:20:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chendave, neolit123

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:

~~cmd/kubeadm/OWNERS~~ [neolit123]

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

chendave · 2023-08-30T07:21:43Z

please always prefix kubeadm release notes with kubeadm:

Done.

chendave · 2023-08-30T07:24:25Z

/hold

testcase need to be updated as well.

…`reset` and `join` component configs is only needed for `kubeadm init`, the `join` and `reset` doesn't need to provid the config with component configs. Signed-off-by: Dave Chen <dave.chen@arm.com>

chendave · 2023-08-30T07:39:25Z

/unhold

updated testcases since the component-configs flags is now not available for reset and join.

pacoxu · 2023-08-30T08:16:23Z

/lgtm

k8s-ci-robot · 2023-08-30T08:16:30Z

LGTM label has been added.

Git tree hash: fd8fc23c0895bd03e652d01fa44e9953367235b5

chendave · 2023-08-30T09:34:26Z

/test pull-kubernetes-e2e-gce

k8s-ci-robot added the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Jul 15, 2023

chendave mentioned this pull request Jul 15, 2023

kubeadm: add kubeadm config print reset-defaults command for ResetConfiguration #119345

Merged

k8s-ci-robot added area/kubeadm sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jul 15, 2023

k8s-ci-robot requested review from pacoxu and SataQiu July 15, 2023 11:25

neolit123 reviewed Jul 17, 2023

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 17, 2023

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 17, 2023

chendave force-pushed the clean_testcases branch from c414e90 to cc163c6 Compare August 7, 2023 03:32

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 7, 2023

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 7, 2023

k8s-ci-robot changed the title ~~kubeadm: Cleanup testcases for ResetConfiguration defaults~~ kubeadm: Deprecate the default values of component config for reset and join Aug 7, 2023

pacoxu reviewed Aug 28, 2023

View reviewed changes

k8s-ci-robot assigned neolit123 Aug 29, 2023

neolit123 reviewed Aug 29, 2023

View reviewed changes

chendave force-pushed the clean_testcases branch from 072ab51 to 6e62f8f Compare August 30, 2023 07:07

chendave changed the title ~~kubeadm: Deprecate the default values of component config for reset and join~~ kubeadm: Remove the usage to print the default component configs for reset and join Aug 30, 2023

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Aug 30, 2023

k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Aug 30, 2023

neolit123 reviewed Aug 30, 2023

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 30, 2023

kubeadm: Remove the usage to print the default component configs for …

879dad9

…`reset` and `join` component configs is only needed for `kubeadm init`, the `join` and `reset` doesn't need to provid the config with component configs. Signed-off-by: Dave Chen <dave.chen@arm.com>

chendave force-pushed the clean_testcases branch from 6e62f8f to 879dad9 Compare August 30, 2023 07:38

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 30, 2023

k8s-ci-robot assigned pacoxu Aug 30, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 30, 2023

k8s-ci-robot merged commit 3cf3702 into kubernetes:master Aug 30, 2023
14 checks passed

k8s-ci-robot added this to the v1.29 milestone Aug 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kubeadm: Remove the usage to print the default component configs for `reset` and `join` #119346

kubeadm: Remove the usage to print the default component configs for `reset` and `join` #119346

chendave commented Jul 15, 2023 •

edited

k8s-ci-robot commented Jul 15, 2023

SataQiu commented Jul 16, 2023

neolit123 commented Jul 16, 2023

SataQiu commented Jul 16, 2023

neolit123 left a comment

pacoxu commented Jul 17, 2023 •

edited

chendave commented Jul 17, 2023

chendave commented Aug 7, 2023

chendave commented Aug 7, 2023

pacoxu Aug 28, 2023 •

edited

chendave Aug 28, 2023

dims commented Aug 29, 2023

neolit123 Aug 29, 2023 •

edited

neolit123 left a comment

chendave commented Aug 30, 2023

chendave commented Aug 30, 2023 •

edited

pacoxu commented Aug 30, 2023 •

edited

chendave commented Aug 30, 2023

chendave commented Aug 30, 2023

neolit123 left a comment

k8s-ci-robot commented Aug 30, 2023

chendave commented Aug 30, 2023

chendave commented Aug 30, 2023

chendave commented Aug 30, 2023

pacoxu commented Aug 30, 2023

k8s-ci-robot commented Aug 30, 2023

chendave commented Aug 30, 2023

kubeadm: Remove the usage to print the default component configs for reset and join #119346

kubeadm: Remove the usage to print the default component configs for reset and join #119346

Conversation

chendave commented Jul 15, 2023 • edited

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot commented Jul 15, 2023

SataQiu commented Jul 16, 2023

neolit123 commented Jul 16, 2023

SataQiu commented Jul 16, 2023

neolit123 left a comment

Choose a reason for hiding this comment

pacoxu commented Jul 17, 2023 • edited

chendave commented Jul 17, 2023

chendave commented Aug 7, 2023

chendave commented Aug 7, 2023

pacoxu Aug 28, 2023 • edited

Choose a reason for hiding this comment

chendave Aug 28, 2023

Choose a reason for hiding this comment

dims commented Aug 29, 2023

neolit123 Aug 29, 2023 • edited

Choose a reason for hiding this comment

neolit123 left a comment

Choose a reason for hiding this comment

chendave commented Aug 30, 2023

chendave commented Aug 30, 2023 • edited

pacoxu commented Aug 30, 2023 • edited

chendave commented Aug 30, 2023

chendave commented Aug 30, 2023

neolit123 left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Aug 30, 2023

chendave commented Aug 30, 2023

chendave commented Aug 30, 2023

chendave commented Aug 30, 2023

pacoxu commented Aug 30, 2023

k8s-ci-robot commented Aug 30, 2023

chendave commented Aug 30, 2023

kubeadm: Remove the usage to print the default component configs for `reset` and `join` #119346

kubeadm: Remove the usage to print the default component configs for `reset` and `join` #119346

chendave commented Jul 15, 2023 •

edited

pacoxu commented Jul 17, 2023 •

edited

pacoxu Aug 28, 2023 •

edited

neolit123 Aug 29, 2023 •

edited

chendave commented Aug 30, 2023 •

edited

pacoxu commented Aug 30, 2023 •

edited