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

Migrates addons from RCs to Deployments #36008

Merged
merged 4 commits into from
Nov 10, 2016
Merged

Migrates addons from RCs to Deployments #36008

merged 4 commits into from
Nov 10, 2016

Conversation

MrHohn
Copy link
Member

@MrHohn MrHohn commented Nov 1, 2016 •
edited
Loading

Fixes #33698.

Below addons are being migrated:

  • kube-dns
  • GLBC default backend
  • Dashboard UI
  • Kibana

For the new deployments, the version suffixes are removed from their names. Version related labels are also removed because they are confusing and not needed any more with regard to how Deployment and the new Addon Manager works.

The replica field in kube-dns Deployment manifest is removed for the incoming DNS horizontal autoscaling feature #33239.

The replica field in Dashboard Deployment manifest is also removed because the rescheduler e2e test is manually scaling it.

Some resource limit related fields in heapster-controller.yaml are removed, as they will be set up by the addon resizer containers. Detailed reasons in #34513.

Three e2e tests are modified:

  • rescheduler.go: Changed to resize Dashboard UI Deployment instead of ReplicationController.
  • addon_update.go: Some namespace related changes in order to make it compatible with the new Addon Manager.
  • dns_autoscaling.go: Changed to examine kube-dns Deployment instead of ReplicationController.

Both of above two tests passed on my own cluster. The upgrade process --- from old Addons with RCs to new Addons with Deployments --- was also tested and worked as expected.

The last commit upgrades Addon Manager to v6.0. It is still a work in process and currently waiting for #35220 to be finished. (The Addon Manager image in used comes from a non-official registry but it mostly works except some corner cases.)

@piosz @gmarek could you please review the heapster part and the rescheduler test?

@mikedanese @thockin

cc @kubernetes/sig-cluster-lifecycle

Notes:

  • Kube-dns manifest still uses *-rc.yaml for the new Deployment. The stale file names are preserved here for receiving faster review. May send out PR to re-organize kube-dns's file names after this.
  • Heapster Deployment's name remains in the old fashion(with -v1.2.0 suffix) for avoiding describe this upgrade transition explicitly. In this way we don't need to attach fake apply labels to the old Deployments.

This change is Reviewable

@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Nov 1, 2016
@MrHohn
Copy link
Member Author

MrHohn commented Nov 1, 2016

@k8s-bot node e2e test this, issue: #32430

@MrHohn
Copy link
Member Author

MrHohn commented Nov 1, 2016

@k8s-bot gci gke e2e test this, issue: #33388

@MrHohn
Copy link
Member Author

MrHohn commented Nov 2, 2016

@k8s-bot node e2e test this, issue: #32430

@k8s-ci-robot
Copy link
Contributor

Jenkins GCE Node e2e failed for commit 745d306. Full PR test history.

The magic incantation to run this job again is @k8s-bot node e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@MrHohn
Copy link
Member Author

MrHohn commented Nov 2, 2016

@k8s-bot node e2e test this, issue: #32430

thockin
thockin requested changes Nov 2, 2016
View reviewed changes
cluster/addons/dns/skydns-rc.yaml.base
kubernetes.io/cluster-service: "true"
spec:
replicas: __PILLAR__DNS__REPLICAS__
Copy link
Member

Choose a reason for hiding this comment

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

Where does the initial replica count live now?

Maybe leave a comment here specifically saying # replicas: not specified here, see xyz.yaml or maybe even specify why.

Copy link
Member

Choose a reason for hiding this comment

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

Same for other DNS files :)

Copy link
Member Author

Choose a reason for hiding this comment

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

They live nowhere for now. I want to hand over this responsibility to the DNS horizontal autoscaler.

As for this RC -> Deployment migration, kube-dns might rely on the dns autoscaler in order to be re-scaled to a proper number, otherwise the new kube-dns Deployment would have 1 replica by default. For the future upgrades(Deployment -> Deployment), the replica count would not got clobbered anymore.

cluster/saltbase/salt/kube-addons/kube-addon-manager.yaml
spec:
hostNetwork: true
containers:
- name: kube-addon-manager
# When updating version also bump it in cluster/images/hyperkube/static-pods/addon-manager.json
image: gcr.io/google-containers/kube-addon-manager:v5.2
image: gcr.io/zihongz-k8s-dev/kube-addon-manager:v6.0
Copy link
Member

Choose a reason for hiding this comment

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

don';t forget to change this back!

@thockin thockin assigned mikedanese, bprashanth and piosz and unassigned zmerlynn Nov 2, 2016
bryk
bryk approved these changes Nov 3, 2016
View reviewed changes
Copy link
Contributor

@bryk bryk left a comment

Choose a reason for hiding this comment

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

LGTM Dashboard UI changes

@MrHohn MrHohn changed the title [wip] Migrates addons from RCs to Deployments Migrates addons from RCs to Deployments Nov 4, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins Kubemark GCE e2e failed for commit 1b6e6bd9a081e7793004cc1e8ee7a0b313d180e3. Full PR test history.

The magic incantation to run this job again is @k8s-bot kubemark e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@MrHohn
Copy link
Member Author

MrHohn commented Nov 4, 2016

Thanks @bryk for reviewing.

Bumped up Addon Manager to v6.0-alpha.1 and removed WIP. Will send another PR to bump up Addon Manager to v6.0 after kubernetes v1.5.0.beta is cut.

It would be great if this could got LGTM earlier in order to test the new Addon Manager and addons over this weekend. @mikedanese @piosz @bprashanth

@saad-ali
Copy link
Member

saad-ali commented Nov 8, 2016

Release-czar approved post-code freeze merge--This was LGTMed and in the merge-queue at code freeze time for 1.5. Leaving 1.5 milestone to let it gets merged after code freeze.

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 9, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins GKE smoke e2e failed for commit fe3a0d2. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-github-robot k8s-github-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 9, 2016
@MrHohn
Copy link
Member Author

MrHohn commented Nov 9, 2016

@k8s-bot unit test this, issue: #36473

@MrHohn
Copy link
Member Author

MrHohn commented Nov 9, 2016

@k8s-bot cvm gke e2e test this, issue: #33388

@k8s-ci-robot
Copy link
Contributor

Jenkins unit/integration failed for commit fe3a0d2. Full PR test history.

The magic incantation to run this job again is @k8s-bot unit test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@MrHohn
Copy link
Member Author

MrHohn commented Nov 9, 2016

@k8s-bot unit test this, issue: #36473

@MrHohn
Copy link
Member Author

MrHohn commented Nov 9, 2016

Rebased and passed tests.

@bprashanth bprashanth added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 9, 2016
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit c98fc70 into kubernetes:master Nov 10, 2016
@krousey krousey mentioned this pull request Nov 16, 2016
Closed
@krousey krousey mentioned this pull request Nov 29, 2016
Closed
@MrHohn MrHohn mentioned this pull request Dec 1, 2016
Merged
@chentao1596 chentao1596 mentioned this pull request Dec 5, 2016
Closed
This was referenced Dec 6, 2016
Closed
Closed
Closed
Closed
Closed
@MrHohn MrHohn deleted the addon-rc-migrate branch May 16, 2017 23:54
@MrHohn MrHohn mentioned this pull request Jan 26, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bprashanth bprashanth bprashanth left review comments

@thockin thockin thockin requested changes

@bryk bryk bryk approved these changes

Assignees

@mikedanese mikedanese

@piosz piosz

@bprashanth bprashanth

Labels
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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.5
Development

Successfully merging this pull request may close these issues.

Migrating addons from RCs to Deployments.