Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Splitting master/node services into separate charm layers #40324
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
label
Jan 23, 2017
k8s-reviewable
commented
Jan 23, 2017
k8s-merge-robot
assigned
mbruzek
Jan 23, 2017
k8s-merge-robot
added
size/XXL
release-note
labels
Jan 23, 2017
|
Additional amended update regarding missing headers in python files has been addressed per bazel check. Waiting on additional automated pass/failure notifications. |
|
Jenkins Bazel Build failed for commit 18617ad. Full PR test history. cc @chuckbutler, your PR dashboard The magic incantation to run this job again is 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. |
|
Jenkins GKE smoke e2e failed for commit 18617ad. Full PR test history. cc @chuckbutler, your PR dashboard The magic incantation to run this job again is 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. |
|
Jenkins verification failed for commit 18617ad. Full PR test history. cc @chuckbutler, your PR dashboard The magic incantation to run this job again is 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. |
|
Jenkins kops AWS e2e failed for commit 18617ad. Full PR test history. cc @chuckbutler, your PR dashboard The magic incantation to run this job again is 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. |
|
[APPROVALNOTIFIER] Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
marcoceppi
referenced this pull request
Jan 24, 2017
Closed
Fix and sync kube-dns multiarch template in juju. #39899
|
Ok, some minor feedback here. The boilerplate checker is looking for a -newline- in the boilerplate at the end of the file. This will cause us heartburn during lint checks, but now passes the bazel boilerplate checker. I expect the rest will start to fall in line now. |
|
Jenkins GCI GKE smoke e2e failed for commit 3fcf279. Full PR test history. cc @chuckbutler, your PR dashboard The magic incantation to run this job again is 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. |
|
Looking at the most recent GKI failure, it appears that it flaked when connecting to the infra. I'd say this was an infra related issue and not a code quality issue. (plus we didn't touch core code that would trigger this failure <3) |
|
Looks like we're green in most cases outside of the test infra failure \o/ |
|
I was assigned as the reviewer on this PR, but full disclosure was involved in some of these commits. The files changed are limited to the We put a lot of effort in to get the e2e tests running against a Juju charm cluster and have our successful test results uploaded to the gubernator daily. https://k8s-gubernator.appspot.com/builds/canonical-kubernetes-tests/logs/kubernetes-gce-e2e-node/ With these successful test results I have confidence that these changes are safe for the cluster directory. |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
label
Jan 24, 2017
|
/approve |
|
cc @mikedanese - I'm at a bit of a loss for self help here. We're listed in the OWNERS file as approvers, but were concerned that since we did the lions-share of the work here, its likely to be a conflict with the bot. Is that the case or are we misinformed at how the review/approve tier works? |
|
/approve |
|
We're hoping to use some personal responsibility and personal accountability here. If you feel you are too close to bring a neutral approval eye then don't do it and find someone else. If you really think you can safely decide if (someone else's) patch makes sense then do it. I'm not afraid to approve PRs from other people at my company. But if I think it might be controversial at all I'd likely not do so. |
k8s-merge-robot
added
the
approved
label
Jan 24, 2017
|
Thank you for the encouragement @eparis. We've thoroughly tested this and feel it's a good contribution. We had some small reservations about how it would be perceived by project contributors if we were self-serving, but it sounds like that's the right path to be on at this time. We've owned this directory tree and are also updating the docs in lockstep with our work. (in this instance, docs landed first, which was a win!) Appreciate the follow-up and approval :) |
This was referenced Jan 24, 2017
|
Automatic merge from submit-queue (batch tested with PRs 40335, 40320, 40324, 39103, 40315) |
chuckbutler commentedJan 23, 2017
•
Edited 1 time
-
chuckbutler
Jan 23, 2017
What this PR does / why we need it:
This branch includes a roll-up series of commits from a fork of the
Kubernetes repository pre 1.5 release because we didn't make the code freeze.
This additional effort has been fully tested and has results submit into
the gubernator to enhance confidence in this code quality vs. the single
layer, posing as both master/node.
To reference the gubernator results, please see:
https://k8s-gubernator.appspot.com/builds/canonical-kubernetes-tests/logs/kubernetes-gce-e2e-node/
Apologies in advance for the large commit however, we did not want to
submit without having successful upstream automated testing results.
This commit includes:
drift!)
Special notes for your reviewer:
Additional note: We will be targeting -all- future work against upstream
so large pull requests of this magnitude will not occur again.
Release note: