Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update kubeadm reference for v1.30 #45911

Merged
merged 1 commit into from Apr 18, 2024

Conversation

tengqm
Copy link
Contributor

@tengqm tengqm commented Apr 18, 2024

No description provided.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 18, 2024
@k8s-ci-robot k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Apr 18, 2024
Copy link

netlify bot commented Apr 18, 2024

Pull request preview available for checking

Name Link
🔨 Latest commit 1da3e56
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/6620f1bfdd86e80008942bfc
😎 Deploy Preview https://deploy-preview-45911--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@sftim
Copy link
Contributor

sftim commented Apr 18, 2024

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sftim

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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 18, 2024
Copy link
Member

@neolit123 neolit123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/kubernetes/kubernetes/pull/123372/files#diff-6799fb75a0f7b206ad9df4784047d69309b89001778ff5b3be5c155431280e5fR33

@carlory @pacoxu
the above is not correct. it forces the kubeadm check expiration command to import some flags that we don't want.

@@ -30,6 +30,13 @@ kubeadm certs check-expiration [flags]
</colgroup>
<tbody>

<tr>
<td colspan="2">--allow-missing-template-keys&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;Default: true</td>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should not be a kubeadm flag, we need to understand what is going on here.
https://github.com/search?q=repo%3Akubernetes%2Fkubernetes%20allow-missing-template-keys&type=code

@@ -44,6 +51,13 @@ kubeadm certs check-expiration [flags]
<td></td><td style="line-height: 130%; word-wrap: break-word;"><p>Path to a kubeadm configuration file.</p></td>
</tr>

<tr>
<td colspan="2">-o, --experimental-output string&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;Default: "text"</td>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@@ -58,6 +72,13 @@ kubeadm certs check-expiration [flags]
<td></td><td style="line-height: 130%; word-wrap: break-word;"><p>The kubeconfig file to use when talking to the cluster. If the flag is not set, a set of standard locations can be searched for an existing kubeconfig file.</p></td>
</tr>

<tr>
<td colspan="2">--show-managed-fields</td>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto.

@neolit123
Copy link
Member

neolit123 commented Apr 18, 2024

https://github.com/kubernetes/kubernetes/pull/123372/files#diff-6799fb75a0f7b206ad9df4784047d69309b89001778ff5b3be5c155431280e5fR33

@carlory @pacoxu the above is not correct. it forces the kubeadm check expiration command to import some flags that we don't want.

looks like all kubeadm commands with structured output now have these extra flags allow-missing-template-keys , --show-managed-fields

also wasn't --experimental-output just --output?

i'm trying to find where these changes happened.

$ kubeadm config images list --help

Print a list of images kubeadm will use. The configuration file is used in case any images or image repositories are customized

Usage:
  kubeadm config images list [flags]

Flags:
      --allow-missing-template-keys   If true, ignore any errors in templates when a field or map key is missing in the template. Only applies to golang and jsonpath output formats. (default true)
      --config string                 Path to a kubeadm configuration file.
  -o, --experimental-output string    Output format. One of: text|json|yaml|go-template|go-template-file|template|templatefile|jsonpath|jsonpath-as-json|jsonpath-file. (default "text")
      --feature-gates string          A set of key=value pairs that describe feature gates for various features. Options are:
                                      EtcdLearnerMode=true|false (BETA - default=true)
                                      PublicKeysECDSA=true|false (DEPRECATED - default=false)
                                      RootlessControlPlane=true|false (ALPHA - default=false)
                                      UpgradeAddonsBeforeControlPlane=true|false (DEPRECATED - default=false)
                                      WaitForAllControlPlaneComponents=true|false (ALPHA - default=false)
  -h, --help                          help for list
      --image-repository string       Choose a container registry to pull control plane images from (default "registry.k8s.io")
      --kubernetes-version string     Choose a specific Kubernetes version for the control plane. (default "stable-1")
      --show-managed-fields           If true, keep the managedFields when printing objects in JSON or YAML format.

Global Flags:
      --add-dir-header      If true, adds the file directory to the header of the log messages
      --kubeconfig string   The kubeconfig file to use when talking to the cluster. If the flag is not set, a set of standard locations can be searched for an existing kubeconfig file. (default "/etc/kubernetes/admin.conf")
      --rootfs string       [EXPERIMENTAL] The path to the 'real' host root filesystem.
      --skip-headers        If true, avoid header prefixes in the log messages
  -v, --v Level             number for the log level verbosity

@neolit123
Copy link
Member

https://github.com/kubernetes/kubernetes/pull/123372/files#diff-6799fb75a0f7b206ad9df4784047d69309b89001778ff5b3be5c155431280e5fR33
@carlory @pacoxu the above is not correct. it forces the kubeadm check expiration command to import some flags that we don't want.

looks like all kubeadm commands with structured output now have these extra flags allow-missing-template-keys , --show-managed-fields

also wasn't --experimental-output just --output?

i'm trying to find where these changes happened.

never mind, it seems this has been the case:
https://github.com/carlory/kubernetes/blob/227c2e7c2b2c05a9c8b2885460e28e4da25cf558/cmd/kubeadm/app/util/output/output.go#L102-L108

so we never graduated our --experimental-output flag to just --output. i will log a ticket for 1.31 for that.
and we explicitly add the extra template / json flags.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 18, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 0964ec5dc81b547a635544042efe3ef91e227ef1

@k8s-ci-robot k8s-ci-robot merged commit 5a6486c into kubernetes:main Apr 18, 2024
6 checks passed
@carlory
Copy link
Member

carlory commented Apr 18, 2024

@neolit123 The --allow-missing-template-keys flag exists in 1.29.

(base) (⎈|kind-kind:test)➜  kubernetes git:(55019c83b0f) git checkout v1.29.4
(base) (⎈|kind-kind:test)➜  kubernetes git:(55019c83b0f) go run cmd/kubeadm/kubeadm.go config images list -h | grep out
      --allow-missing-template-keys   If true, ignore any errors in templates when a field or map key is missing in the template. Only applies to golang and jsonpath output formats. (default true)
  -o, --experimental-output string    Output format. One of: text|json|yaml|go-template|go-template-file|template|templatefile|jsonpath|jsonpath-as-json|jsonpath-file. (default "text")
      --one-output               If true, only write logs to their native severity level (vs also writing to each lower severity level; no effect when -logtostderr=true)

when I (re)implement kubernetes/kubernetes#123372 and
kubernetes/kubernetes#123578, I follow the config images list code style.

According to kubernetes/kubernetes#123372, it accepts Go template as output.

@neolit123
Copy link
Member

yes, i got confused.
logged this issue to graduate experimental-output to output in the next 2 releases
kubernetes/kubeadm#3045

@tengqm tengqm deleted the kubeadm-ref-1.30 branch April 19, 2024 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants