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

Updating federation up script to create secrets with federation-apiserver and k8s apiservers kubeconfigs #26914

Merged
2 commits merged into from
Jun 8, 2016

Conversation

nikhiljindal
Copy link
Contributor

Follow up to #26819

cc @kubernetes/sig-cluster-federation

@nikhiljindal
Copy link
Contributor Author

Am still testing it out

@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-label-needed labels Jun 6, 2016
@ghost ghost self-assigned this Jun 6, 2016
@ghost
Copy link

ghost commented Jun 7, 2016

@nikhiljindal Let me know as soon as it's ready for review.

@ghost ghost added this to the v1.3 milestone Jun 7, 2016
@ghost ghost added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note-none Denotes a PR that doesn't merit a release note. area/cluster-federation and removed release-note-label-needed labels Jun 7, 2016
@@ -146,6 +147,21 @@ function create-federated-api-objects {
sleep 4
done

# Create a kubeconfig with credentails for federation-apiserver and create a
# secret for it.
Copy link

@ghost ghost Jun 7, 2016

Choose a reason for hiding this comment

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

nit: This comment does not seem to apply to the next line of code?

@ghost
Copy link

ghost commented Jun 7, 2016

Minor nit, otherwise LGTM.

@ghost ghost added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 7, 2016
@nikhiljindal
Copy link
Contributor Author

Sorry I havent been able to get this to work yet.

@nikhiljindal nikhiljindal removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 7, 2016
@nikhiljindal
Copy link
Contributor Author

Here is the problem I am having:
When I call create-kubeconfig with a different value to create a kubeconfig file with just the federation-apiserver credentials, create-kubeconfig exports KUBECONFIG with this new value and hence all future kubectl commands are reading this new kubeconfig file which can only talk to federation-apiserver and hence many kubectl commands are failing.
I am fixing it so that create-kubeconfig does not export KUBECONFIG with a new value.

@nikhiljindal nikhiljindal force-pushed the FedAPIServerSecret branch 2 times, most recently from 0adc75c to d2d3697 Compare June 7, 2016 04:32
@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 7, 2016
@nikhiljindal
Copy link
Contributor Author

Updated the code.

I am still having trouble with pushing my images, so federation-apiserver and federation-controller-manager are not coming up for me.
But verified that the script creates the right secret and it is available for the controller manager to use.

@quinton-hoole PTAL. Should be ready to be merged

function clear-kubeconfig() {
export KUBECONFIG=${KUBECONFIG:-$DEFAULT_KUBECONFIG}
OVERRIDE_CONTEXT=${OVERRIDE_CONTEXT:-}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an independent change. Without this change, running hack/e2e.go --up followed by hack/e2e.go --down would leave contexts in kubeconfig that can affect later runs. This change ensures that kubeconfig is cleared correctly.

@nikhiljindal
Copy link
Contributor Author

@k8s-bot e2e test this issue: #23545

@k8s-bot node e2e test this issue: #26431

@nikhiljindal
Copy link
Contributor Author

cc @colhom

@nikhiljindal
Copy link
Contributor Author

nikhiljindal commented Jun 7, 2016

Same errors happening again. Unrelated to this PR

@nikhiljindal
Copy link
Contributor Author

Trying again
@k8s-bot e2e test this issue: #23545

@k8s-bot node e2e test this issue: #26431

@nikhiljindal
Copy link
Contributor Author

@k8s-bot node e2e test this issue: #26431

@ghost
Copy link

ghost commented Jun 7, 2016

LGTM!

@ghost ghost added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 7, 2016
@nikhiljindal
Copy link
Contributor Author

Added another commit to create secrets for the kubernetes apiservers as well.
Also updated the logic to delete these generated secrets in cleanup.

@quinton-hoole PTAL.

@nikhiljindal nikhiljindal changed the title Updating federation up script to create a secret with federation-apiserver kubeconfig Updating federation up script to create secrets with federation-apiserver and k8s apiservers kubeconfigs Jun 8, 2016
@nikhiljindal
Copy link
Contributor Author

Note that I am creating secrets for k8s clusters only on GCE for now.
Doing the same on vagrant seems straightforward, but I am not so sure about GKE.

In GKE script, we directly call gcloud create cluster to create the cluster: https://github.com/kubernetes/kubernetes/blob/master/cluster/gke/util.sh#L183. I guess that updates the kubeconfig as well.

@nikhiljindal
Copy link
Contributor Author

This PR with GCE only secrets should atleast unblock @madhusudancs for his service controller tests.

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 8, 2016
@ghost
Copy link

ghost commented Jun 8, 2016

Awesome. LGTM.

@ghost ghost added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 8, 2016
@nikhiljindal nikhiljindal mentioned this pull request Jun 8, 2016
20 tasks
@ghost
Copy link

ghost commented Jun 8, 2016

@k8s-oncall @yujuhong could we merge this manually please? It's passed all checks, but the http://submit-queue.k8s.io/#/queue is currently stalled for unrelated reasons. This PR is blocking further e2e testing of cluster federation, which is in turn blocking the v1.3 launch. Thanks!

@ghost
Copy link

ghost commented Jun 8, 2016

Subsequent unit test failure looks like a flake. #27024

@k8s-bot test this issue: #27024

@madhusudancs
Copy link
Contributor

@nikhiljindal Thank you very much!

@k8s-bot
Copy link

k8s-bot commented Jun 8, 2016

GCE e2e build/test passed for commit 9443bf0.

@ghost
Copy link

ghost commented Jun 8, 2016

Manually merging. See note above for rationale,

@ghost ghost merged commit c578678 into kubernetes:master Jun 8, 2016
k8s-github-robot pushed a commit that referenced this pull request Jun 21, 2016
Automatic merge from submit-queue

federation: Creating kubeconfig files to be used for creating secrets for clusters on aws and gke

Extension of #26914 which created the kubeconfig files for gce clusters.
This PR extends it to AWS, vagrant and GKE.

The change for AWS and vagrant is exactly same as GCE.
For GKE, since `gcloud create clusters` creates kubeconfig, we are just copying the generated kubeconfig to the desired location

cc @kubernetes/sig-cluster-federation @colhom 
@roberthbailey for GKE
perotinus pushed a commit to kubernetes-retired/cluster-registry that referenced this pull request Sep 2, 2017
Automatic merge from submit-queue

federation: Creating kubeconfig files to be used for creating secrets for clusters on aws and gke

Extension of kubernetes/kubernetes#26914 which created the kubeconfig files for gce clusters.
This PR extends it to AWS, vagrant and GKE.

The change for AWS and vagrant is exactly same as GCE.
For GKE, since `gcloud create clusters` creates kubeconfig, we are just copying the generated kubeconfig to the desired location

cc @kubernetes/sig-cluster-federation @colhom 
@roberthbailey for GKE
This pull request was closed.
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. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants