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.

Sign up for GitHub

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

Jump to bottom

kubeadm: remove kubeadm config print-defaults #71467

Merged
merged 1 commit into from
Dec 4, 2018
Merged

kubeadm: remove kubeadm config print-defaults #71467

merged 1 commit into from
Dec 4, 2018

Conversation

rosti
Copy link
Contributor

@rosti rosti commented Nov 27, 2018

What type of PR is this?
/kind cleanup

What this PR does / why we need it:

Remove the old and deprecated kubeadm config print-defaults command.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
refs kubernetes/kubeadm#1152

Special notes for your reviewer:

This one is for 1.14.

/cc @kubernetes/sig-cluster-lifecycle-pr-reviews
/area kubeadm
/assign @fabriziopandini
/assign @timothysc

Does this PR introduce a user-facing change?:

kubeadm: remove deprecated kubeadm config print-defaults command

@k8s-ci-robot k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Nov 27, 2018
@k8s-ci-robot k8s-ci-robot added sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. area/kubeadm size/L Denotes a PR that changes 100-499 lines, ignoring generated files. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 27, 2018
@neolit123
Copy link
Member

/priority important-longterm

@k8s-ci-robot k8s-ci-robot added priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Nov 27, 2018
timothysc
timothysc approved these changes Nov 27, 2018
View reviewed changes
Copy link
Member

@timothysc timothysc left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

I'm kind of surprised/saddened that there were not tests associated with it.

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 27, 2018
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

1 similar comment
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@xiangpengzhao
Copy link
Contributor

pull-kubernetes-verify job says:

I1128 01:49:10.759] < docs/man/man1/kubeadm-config-print-default.1
I1128 01:49:10.759] List of generated docs doesn't match a freshly built list. Please run hack/update-generated-docs.sh

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

1 similar comment
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@xiangpengzhao
Copy link
Contributor

/hold
@rosti feel free to hold cancel after you fix the test failure :)

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 28, 2018
@rosti rosti force-pushed the nuke-config-print-defaults branch from 4efb568 to 70bd6e5 Compare December 3, 2018 09:37
@k8s-ci-robot k8s-ci-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Dec 3, 2018
@rosti
Copy link
Contributor Author

rosti commented Dec 3, 2018

Thanks @xiangpengzhao ! Should be OK now.
/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 3, 2018
@xiangpengzhao
Copy link
Contributor

/retest

@rosti
Copy link
Contributor Author

rosti commented Dec 3, 2018

/test pull-kubernetes-e2e-gce

yagonobre
yagonobre approved these changes Dec 3, 2018
View reviewed changes
Copy link
Member

@yagonobre yagonobre left a comment

Choose a reason for hiding this comment

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

/lgtm

cmd/kubeadm/app/cmd/config.go Outdated
return []byte{}, errors.Errorf("--api-object needs to be one of %v", getAllAPIObjectNames())
}
return getDefaultComponentConfigBytes(registration)
realobj, ok := registration.GetFromInternalConfig(&defaultedInitConfig.ClusterConfiguration)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe use realObj or just obj

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for spotting this one @yagonobre ! Just fixed it.

Copy link
Member

Choose a reason for hiding this comment

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

Nice!

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 3, 2018
@rosti
kubeadm: remove kubeadm config print-defaults
ffb670b 
Signed-off-by: Rostislav M. Georgiev <rostislavg@vmware.com>
@rosti rosti force-pushed the nuke-config-print-defaults branch from 70bd6e5 to ffb670b Compare December 3, 2018 15:44
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 3, 2018
@yagonobre
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 3, 2018
@yagonobre
Copy link
Member

/test pull-kubernetes-integration

@rosti
Copy link
Contributor Author

rosti commented Dec 3, 2018

/test pull-kubernetes-e2e-kops-aws
/test pull-kubernetes-verify

@luxas luxas added this to the v1.14 milestone Dec 4, 2018
luxas
luxas approved these changes Dec 4, 2018
View reviewed changes
Copy link
Member

@luxas luxas left a comment

Choose a reason for hiding this comment

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

nit: I think you can delete the // TODO: Probably move to getDefaultedClusterConfig? comment now.

/lgtm
/approve

Thanks @rosti for the cleanup!

@luxas luxas added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 4, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: luxas, rosti, timothysc

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

@luxas
Copy link
Member

luxas commented Dec 4, 2018

/retest

1 similar comment
@luxas
Copy link
Member

luxas commented Dec 4, 2018

/retest

@k8s-ci-robot k8s-ci-robot merged commit 7af5238 into kubernetes:master Dec 4, 2018
mrueg added a commit to mrueg/website that referenced this pull request Apr 5, 2019
@mrueg
kubelet-integration: Update kubeadm config command
36d3b80 
print-default subcommand got removed in 1.14

See: kubernetes/kubernetes#71467

Signed-off-by: Manuel Rüger <manuel@rueg.eu>
@mrueg mrueg mentioned this pull request Apr 5, 2019
Merged
k8s-ci-robot pushed a commit to kubernetes/website that referenced this pull request Apr 9, 2019
@mrueg @k8s-ci-robot
kubelet-integration: Update kubeadm config command (#13636)
8f5fbc0 
print-default subcommand got removed in 1.14

See: kubernetes/kubernetes#71467

Signed-off-by: Manuel Rüger <manuel@rueg.eu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yagonobre yagonobre yagonobre approved these changes

@luxas luxas luxas approved these changes

@timothysc timothysc timothysc approved these changes

Assignees

@timothysc timothysc

@fabriziopandini fabriziopandini

@luxas luxas

@yagonobre yagonobre

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/kubeadm cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.14
Development

Successfully merging this pull request may close these issues.
9 participants
@rosti @neolit123 @fejta-bot @xiangpengzhao @yagonobre @k8s-ci-robot @luxas @timothysc @fabriziopandini