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

Automated cherry pick of #62481: kubeadm preflight: check socket path if defined otherwise #63907

Conversation

dixudx
Copy link
Member

@dixudx dixudx commented May 16, 2018

Cherry pick of #62481 on release-1.10.

#62481: kubeadm preflight: check socket path if defined otherwise

Fixes: kubernetes/kubeadm#814

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 16, 2018
@k8s-ci-robot k8s-ci-robot requested review from kad and liztio May 16, 2018 05:40
@k8s-github-robot k8s-github-robot added the do-not-merge/cherry-pick-not-approved Indicates that a PR is not yet approved to merge into a release branch. label May 16, 2018
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. area/kubeadm sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. labels May 16, 2018
@dixudx
Copy link
Member Author

dixudx commented May 16, 2018

/assign timothysc luxas

@MikeSpreitzer @taharah FYI.

@dixudx dixudx force-pushed the automated-cherry-pick-of-#62481-upstream-release-1.10 branch from ec0e4b3 to 5a20494 Compare May 16, 2018 05:51
@MaciekPytel MaciekPytel added cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. and removed do-not-merge/cherry-pick-not-approved Indicates that a PR is not yet approved to merge into a release branch. labels May 16, 2018
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.

/lgtm
/approve

This is okay as a backport, but I think I want to rearrange some of the code at master.

// CommonConfiguration defines the list of common configuration elements and the getter
// methods that must exist for both the MasterConfiguration and NodeConfiguration objects.
// This is used internally to deduplicate the kubeadm preflight checks.
type CommonConfiguration interface {
Copy link
Member

Choose a reason for hiding this comment

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

I haven't made up my mind yet whether this is a good or a bad abstraction at this level for the specific use case, but I can let this pass as a backport for the moment.

@@ -47,6 +48,7 @@ import (
cmoptions "k8s.io/kubernetes/cmd/kube-controller-manager/app/options"
schedulerapp "k8s.io/kubernetes/cmd/kube-scheduler/app"
kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
kubeadmdefaults "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1"
Copy link
Member

Choose a reason for hiding this comment

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

kubeadmapiv1alpha1 in the future please

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dixudx, luxas

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 May 17, 2018
@luxas
Copy link
Member

luxas commented May 17, 2018

/hold
for @detiber or @timothysc to sign off

@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 May 17, 2018
@timothysc
Copy link
Member

I really don't think this seems backport worthy. We should only be backporting fixes that common bug scenarios, otherwise new changes will hit the next release cycle in ~1 month.

@MaciekPytel
Copy link
Contributor

1.10 branch is now frozen for final testing before 1.10.3. Please don't remove hold on this until the release is cut (hopefully on Monday).

@dixudx
Copy link
Member Author

dixudx commented May 21, 2018

/retest

@timothysc
Copy link
Member

I'm still of the opinion that this doesn't meet the criteria for a backport, fwiw.

@MaciekPytel MaciekPytel added do-not-merge/cherry-pick-not-approved Indicates that a PR is not yet approved to merge into a release branch. and removed cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. labels May 24, 2018
@MaciekPytel
Copy link
Contributor

Removing cherrypick-approved based on comment from @timothysc. I'm ok to reapply it once consensus is reached on whether this should be cherrypicked or closed.

/hold cancel
Since the branch is no longer frozen

@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 May 24, 2018
@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


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. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot removed the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 25, 2018
@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label May 25, 2018
@dixudx dixudx force-pushed the automated-cherry-pick-of-#62481-upstream-release-1.10 branch from 5a20494 to 08a95a7 Compare May 29, 2018 08:52
@k8s-ci-robot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels May 29, 2018
@dixudx
Copy link
Member Author

dixudx commented May 29, 2018

Seems the original author has changed his/her email address. The bot is complaining github user account and cncf-cla.

I changed the author mail to mine. But the credit should still gave to @taharah (You can try to sign cncf-cla again using your new email).

@timothysc timothysc added kind/bug Categorizes issue or PR as related to a bug. cherrypick-candidate priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels May 30, 2018
@timothysc timothysc added this to the v1.10 milestone May 30, 2018
@dixudx
Copy link
Member Author

dixudx commented Jun 4, 2018

@luxas @timothysc Needs your LGTM again. Thanks.

@k8s-github-robot
Copy link

[MILESTONENOTIFIER] Milestone Removed From Pull Request

@dixudx @luxas @timothysc @kubernetes/sig-cluster-lifecycle-misc

Important: This pull request was missing the status/approved-for-milestone label for more than 4 days.

Help

@k8s-github-robot k8s-github-robot removed this from the v1.10 milestone Jun 6, 2018
@MaciekPytel
Copy link
Contributor

@dixudx @timothysc @luxas Can you make a decision if this needs to be cherry-picked and lgtm or close this PR?

@dixudx dixudx closed this Aug 1, 2018
@dixudx dixudx deleted the automated-cherry-pick-of-#62481-upstream-release-1.10 branch August 1, 2018 09:08
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. area/kubeadm cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/cherry-pick-not-approved Indicates that a PR is not yet approved to merge into a release branch. kind/bug Categorizes issue or PR as related to a bug. milestone/removed priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. 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
Development

Successfully merging this pull request may close these issues.

6 participants