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

kubeadm: Remove the never-used .Etcd.SelfHosted field #63871

Merged
merged 2 commits into from May 17, 2018

Conversation

@luxas
Copy link
Member

luxas commented May 15, 2018

What this PR does / why we need it:
These API types were added to support the self-hosting etcd feature, which in the end never was merged. Hence these API types are unused and should be removed. Perfect timing to do that is now in our new v1alpha2 scheme.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Part of kubernetes/community#2131

Special notes for your reviewer:
Depends on PRs:

Release note:

kubeadm has removed `.Etcd.SelfHosting` from its configuration API. It was never used in practice.

@kubernetes/sig-cluster-lifecycle-pr-reviews @liztio

@k8s-ci-robot k8s-ci-robot requested review from detiber and xiangpengzhao May 15, 2018

@luxas luxas changed the title Kubeadm remove old selfhosting apis kubeadm: Remove the never-used .Etcd.SelfHosted field May 15, 2018

@luxas luxas force-pushed the luxas:kubeadm_remove_old_selfhosting_apis branch from 0e0d993 to 9048a26 May 16, 2018

@k8s-ci-robot k8s-ci-robot added size/L and removed size/XXL labels May 16, 2018

@luxas

This comment has been minimized.

Copy link
Member Author

luxas commented May 16, 2018

/retest

@liztio

This comment has been minimized.

Copy link
Member

liztio commented May 16, 2018

/lgtm
I like this, taking advantage of v1alpha2 to remove cruft 👍

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented May 16, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liztio, 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

@luxas

This comment has been minimized.

Copy link
Member Author

luxas commented May 16, 2018

That's my evil master plan 😉

@luxas

This comment has been minimized.

Copy link
Member Author

luxas commented May 16, 2018

/retest

@luxas luxas force-pushed the luxas:kubeadm_remove_old_selfhosting_apis branch from 9048a26 to 262c386 May 16, 2018

@k8s-ci-robot k8s-ci-robot removed the lgtm label May 16, 2018

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented May 16, 2018

New changes are detected. LGTM label has been removed.

@luxas

This comment has been minimized.

Copy link
Member Author

luxas commented May 16, 2018

Rebased upon #63866

@luxas luxas added the lgtm label May 16, 2018

@luxas luxas force-pushed the luxas:kubeadm_remove_old_selfhosting_apis branch from 262c386 to 8803ae0 May 16, 2018

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented May 16, 2018

New changes are detected. LGTM label has been removed.

@k8s-ci-robot k8s-ci-robot removed the lgtm label May 16, 2018

@luxas luxas force-pushed the luxas:kubeadm_remove_old_selfhosting_apis branch from 8803ae0 to ce8c2ad May 16, 2018

@luxas luxas added the lgtm label May 16, 2018

@luxas

This comment has been minimized.

Copy link
Member Author

luxas commented May 16, 2018

(fixed up a test)

@luxas

This comment has been minimized.

Copy link
Member Author

luxas commented May 16, 2018

/retest

@fejta-bot

This comment has been minimized.

Copy link

fejta-bot commented May 17, 2018

/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

This comment has been minimized.

Copy link

fejta-bot commented May 17, 2018

/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.

@luxas

This comment has been minimized.

Copy link
Member Author

luxas commented May 17, 2018

/retest

1 similar comment
@luxas

This comment has been minimized.

Copy link
Member Author

luxas commented May 17, 2018

/retest

luxas added some commits May 17, 2018

@luxas luxas force-pushed the luxas:kubeadm_remove_old_selfhosting_apis branch from ce8c2ad to 9633d00 May 17, 2018

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented May 17, 2018

New changes are detected. LGTM label has been removed.

@k8s-ci-robot k8s-ci-robot removed the lgtm label May 17, 2018

@luxas

This comment has been minimized.

Copy link
Member Author

luxas commented May 17, 2018

Somehow the commit I earlier had rebased on was faulty, as one gce test kept failing. However, rebased now again and first now it's green ✔️

@luxas luxas added the lgtm label May 17, 2018

@k8s-github-robot

This comment has been minimized.

Copy link
Contributor

k8s-github-robot commented May 17, 2018

/test all [submit-queue is verifying that this PR is safe to merge]

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented May 17, 2018

@luxas: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-e2e-gce 9633d00 link /test pull-kubernetes-e2e-gce

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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-github-robot

This comment has been minimized.

Copy link
Contributor

k8s-github-robot commented May 17, 2018

Automatic merge from submit-queue (batch tested with PRs 63871, 63927, 63966, 63957, 63844). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit 091b811 into kubernetes:master May 17, 2018

16 of 18 checks passed

pull-kubernetes-e2e-gce Job failed.
Details
Submit Queue Required Github CI test is not green: pull-kubernetes-e2e-gce
Details
cla/linuxfoundation luxas authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gke Skipped
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-local-e2e-containerized Skipped
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details

@luxas luxas referenced this pull request May 19, 2018

Merged

kubeadm: Refactor the .Etcd substruct in the v1alpha2 API #64066

1 of 1 task complete

k8s-github-robot pushed a commit that referenced this pull request May 24, 2018

Kubernetes Submit Queue
Merge pull request #64066 from luxas/kubeadm_etcd_refactor
Automatic merge from submit-queue (batch tested with PRs 64127, 63895, 64066, 64215, 64202). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

kubeadm: Refactor the .Etcd substruct in the v1alpha2 API

**What this PR does / why we need it**:
Splits the monolithic `.Etcd` struct with all the options as fields to a more modular and clear design with two sub-structs for the different modes of hosting etcd we support.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Part of kubernetes/community#2131

**Special notes for your reviewer**:
Depends on:
 - [x] #63917

Follows up: #63871
TODO: I still need to write unit tests for this.

**Release note**:

```release-note
[action required] kubeadm: The `:Etcd` struct has been refactored in the v1alpha2 API. All the options now reside under either `.Etcd.Local` or `.Etcd.External`. Automatic conversions from the v1alpha1 API are supported.
```
@kubernetes/sig-cluster-lifecycle-pr-reviews @liztio
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.