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

Remove etcd2 storage backend #69310

Merged
merged 2 commits into from
Oct 4, 2018
Merged

Remove etcd2 storage backend #69310

merged 2 commits into from
Oct 4, 2018

Conversation

liggitt
Copy link
Member

@liggitt liggitt commented Oct 1, 2018 •
edited
Loading

What this PR does / why we need it:
Removes the deprecated etcd2 backend

Which issue(s) this PR fixes:
xref kubernetes/enhancements#622

Special notes for your reviewer:

Release note:

kube-apiserver has removed support for the `etcd2` storage backend (deprecated since v1.9). Existing clusters must migrate etcd v2 data to etcd v3 storage before upgrading to v1.13.

cc @kubernetes/sig-api-machinery-pr-reviews @kubernetes/sig-cluster-lifecycle-pr-reviews

/assign @lavalamp @deads2k

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Oct 1, 2018
@k8s-ci-robot k8s-ci-robot added sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Oct 1, 2018
@liggitt
Copy link
Member Author

liggitt commented Oct 1, 2018

cc @kubernetes/sig-testing-pr-reviews for impact on test jobs
cc @justinsb

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Oct 1, 2018
@liggitt
Copy link
Member Author

liggitt commented Oct 1, 2018 •
edited
Loading

holding to make sure there are no CI jobs still intentionally starting on etcd2 for test signal (kubernetes/test-infra#7602)

@liggitt liggitt added this to the v1.13 milestone Oct 1, 2018
@liggitt
Copy link
Member Author

liggitt commented Oct 1, 2018

/kind cleanup

@k8s-ci-robot k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Oct 1, 2018
@liggitt liggitt added release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Oct 1, 2018
@fejta
Copy link
Contributor

fejta commented Oct 1, 2018 •
edited
Loading

Do we still have tests that run on etcd2? Is plan to remove them before or after this? And awesome btw :)

@liggitt
Copy link
Member Author

liggitt commented Oct 1, 2018

Do we still have tests that run on etcd2?

I don't know. The only tests we should have at this point are ones that specifically exist to ensure we continue to function on etcd2.

Is plan to remove them before or after this?

They should be removed before this merges.

@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 Oct 1, 2018
timothysc
timothysc approved these changes Oct 1, 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.

/lgtm
/hold

per @fejta 's comment. Once someone cross-verifies test-grid feel free to cancel the hold.

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

justinsb commented Oct 1, 2018

The PRs to get kops not using etcd3 on 1.13 (actually we'll switch on 1.12) are in flight: kubernetes/kops#5820 Just need to update that for the new etcd version - 3.2.24 I believe.

We may be blocked on getting an etcd-management project going as a sig-cluster-lifecycle SIG project, so I'll work on that proposal also.

@BenTheElder
Copy link
Member

see: kubernetes/test-infra#7602

there are a few test-infra things to cleanup when we make the switch. cc @spiffxp

@liggitt
Copy link
Member Author

liggitt commented Oct 3, 2018

/assign @wojtek-t
can you review the "Switch e2e_node to etcd3" commit?

@liggitt liggitt changed the title WIP: Remove etcd2 storage backend Remove etcd2 storage backend Oct 3, 2018
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. labels Oct 3, 2018
@liggitt
Copy link
Member Author

liggitt commented Oct 3, 2018

presubmit tests are green

waiting on @wojtek-t for review of e2e node commit, and @spiffxp for ack that tests look good after kubernetes/test-infra#9672 merged

@wojtek-t
Copy link
Member

wojtek-t commented Oct 3, 2018

can you review the "Switch e2e_node to etcd3" commit?

This looks great.

/lgtm

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

wojtek-t commented Oct 3, 2018

/hold

Since we want to wait for other tests results.
@liggitt - feel free to unhold it when you're ready.

@spiffxp spiffxp mentioned this pull request Oct 3, 2018
Merged
@spiffxp
Copy link
Member

spiffxp commented Oct 3, 2018 •
edited
Loading

/lgtm
I'm confident enough in what I've seen over the past 12 hours to say we're good to go. That said, I waited until overnight PT to let my changes merge, so I got N runs against the same commit. I'm suspect if we see anything as a result of this merging, it will be clear and immediate breakage, rather than flakiness and degredation. I leave it to @liggitt to remove the /hold when ready, and will keep on an on sig-release-master-blocking to start with

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lavalamp, liggitt, spiffxp, timothysc, wojtek-t

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

@liggitt
Copy link
Member Author

liggitt commented Oct 3, 2018

/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 Oct 3, 2018
@liggitt
Copy link
Member Author

liggitt commented Oct 3, 2018

/retest

@liggitt
Copy link
Member Author

liggitt commented Oct 4, 2018

/retest pull-kubernetes-e2e-kops-aws

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

New changes are detected. LGTM label has been removed.

@liggitt
Copy link
Member Author

liggitt commented Oct 4, 2018

rebased

@liggitt liggitt added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 4, 2018
@spiffxp
Copy link
Member

spiffxp commented Oct 4, 2018

/retest

@k8s-ci-robot k8s-ci-robot merged commit 409871e into kubernetes:master Oct 4, 2018
@liggitt liggitt deleted the remove-etcd2 branch October 5, 2018 16:04
@liggitt liggitt mentioned this pull request Oct 8, 2018
Merged
@spiffxp spiffxp mentioned this pull request Oct 8, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timothysc timothysc timothysc approved these changes

@davidopp davidopp Awaiting requested review from davidopp

@deads2k deads2k Awaiting requested review from deads2k

Assignees

@spiffxp spiffxp

@timothysc timothysc

@lavalamp lavalamp

@deads2k deads2k

@wojtek-t wojtek-t

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/apiserver 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. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
v1.13
Development

Successfully merging this pull request may close these issues.

None yet