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

Revert "Revert "Add support for running GCI on the GCE cloud provider"" #25927

Merged
merged 4 commits into from May 23, 2016
Merged

Revert "Revert "Add support for running GCI on the GCE cloud provider"" #25927

merged 4 commits into from May 23, 2016

Conversation

andyzheng0831
Copy link

This is for resubmitting the PR #25425, which was reverted due to breakage in GKE cluster intialization. The root cause is that /etc/gce.conf is incorrectly deleted. Compared with the previous PR, this one corrects the logic.

Test: (1) spin up a k8s cluster and gke cluster and verify kube-system pods all running. (2) Run e2e tests on k8s cluster and gke cluster.

One thing confusing me: two test fail in gke but all pass in oss k8s cluster.

  • Services [It] should be able to create a functioning NodePort service
  • KubeProxy [It] should test kube-proxy [Slow]

The second test case is more strange as the kubeproxy is running on ContainerVM-based node.

@roberthbailey do you know any potential reason for the different result in gke and k8s? For GKE e2e tests, I follow the instruction https://g3doc.corp.google.com/cloud/kubernetes/g3doc/dev/testing.md?cl=head#running-e2es-against-a-gke-cluster. I am not sure if I missed any configuration which led to the failures.

@andyzheng0831 andyzheng0831 added this to the v1.3 milestone May 19, 2016
@k8s-github-robot k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. release-note-label-needed labels May 19, 2016
@andyzheng0831 andyzheng0831 added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels May 19, 2016
@andyzheng0831
Copy link
Author

@k8s-bot unit test this issue #25539

@andyzheng0831
Copy link
Author

@cjcullen told me the failed test cases may be caused by incorrect firewall. My cluster was created using gcloud containers command, instead of jenkins tests. After manually adding the needed firewall rules, the failed test cases all can pass.

It means that this PR can pass both k8s and GKE e2e tests.

@andyzheng0831
Copy link
Author

@k8s-bot unit test this issue #25539

@roberthbailey
Copy link
Contributor

@david-mcmahon -- the original PR had a release note but was reverted. Will it's release note show up in the next release or should this PR have a release note attached?

@andyzheng0831
Copy link
Author

I just updated the PR by splitting it into two commits. One is for reverting the previous reversion of #25425 . Another is for fixing the issue in PR #25425

@roberthbailey
Copy link
Contributor

Marking as e2e-not-required since the merge bot tests won't actually test this code (so there's no point in wasting cycles).

@k8s-github-robot k8s-github-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 20, 2016
@roberthbailey
Copy link
Contributor

lgtm

@david-mcmahon
Copy link
Contributor

@roberthbailey Each PR has its own independent release note state and text.

In the case of an original, a revert and a revert-revert, I'd recommend release-note-none on the first two and some release-note label on the last one with an updated title (or release-note block). You could just leave the release note label on the first PR and add release-note-none to the last two, but ideally you'd want to use the last PR in the chain because it has all the history (for better accounting).

@andyzheng0831
Copy link
Author

I will remove the commit "GCI: add support for CIDR allocator for NodeController" from this PR, as the original PR will be reverted shortly.

@andyzheng0831
Copy link
Author

PR #25997 was abandoned. I add the support for CIDR allocator for NodeController back. The PR is ready

@andyzheng0831
Copy link
Author

@roberthbailey I verified the PR in GKE cluster creation and kube-system pods. Also ran most e2e test cases in GCE and GKE. Please merge it and then let's monitor the gke CI jenkins.

@k8s-bot
Copy link

k8s-bot commented May 23, 2016

GCE e2e build/test passed for commit 6bb0a25.

@roberthbailey
Copy link
Contributor

@k8s-bot unit test this issue: #25967

@roberthbailey roberthbailey added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 23, 2016
@mikedanese mikedanese merged commit 4215fe5 into kubernetes:master May 23, 2016
@andyzheng0831 andyzheng0831 deleted the real-gci-fixed branch June 1, 2016 04:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants