Skip to content

Remove deps on GKE_CLUSTER_NAME,GKE_CLUSTER_ZONE from gke/kubeconfig.sh#242

Open
viccuad wants to merge 2 commits into
masterfrom
viccuad-fix-gke-kubeconfig
Open

Remove deps on GKE_CLUSTER_NAME,GKE_CLUSTER_ZONE from gke/kubeconfig.sh#242
viccuad wants to merge 2 commits into
masterfrom
viccuad-fix-gke-kubeconfig

Conversation

@viccuad

@viccuad viccuad commented Jun 26, 2020

Copy link
Copy Markdown
Member

Importing a kubeconfig should always only depend on the kubeconfig, and general
public cloud credentials
.

It should not depend on specifics of the cluster to be imported (GKE_CLUSTER_NAME or GKE_CLUSTER_ZONE). Because when importing just from a kubeconfig, one doesn't know any of those.

Depending on specifics such as those breaks the case of importing your own
kubeconfigs from the pool into the CI, since it's impossible for the CI to know
the name of the cluster you are importing (nor it needs it anyways).

Hence, this removes them.

Tested by importing a kubeconfig from cf-ci-pools, and seeing that it works,
kubectl work, and everything is fine.

If not:
- call to `err` in line 10 will fail
- we will not be inside of buildfolder, and we will lack the binary for yq
@viccuad
viccuad requested review from greygoo and prabalsharma June 26, 2020 10:09
…econfig.sh

Importing a kubeconfig should always only depend on the kubeconfig, and general
public cloud credentials.

It should not depend on specifics of the cluster to be
imported (GKE_CLUSTER_NAME or GKE_CLUSTER_ZONE). Because when importing just
from a kubeconfig, one doesn't know any of those.

Depending on specifics such as those breaks the case of importing your own
kubeconfigs from the pool into the CI, since it's impossible for the CI to know
the name of the cluster you are importing (nor it needs it anyways).

Hence, this removes them.

Tested by importing a kubeconfig from cf-ci-pools, and seeing that it works,
`kubectl` work, and everything is fine.
@viccuad viccuad changed the title Fix gke/kubeconfig.sh, we need to source scripts before anything Fix gke/kubeconfig.sh Jun 26, 2020
@viccuad viccuad changed the title Fix gke/kubeconfig.sh Remove deps on GKE_CLUSTER_NAME,GKE_CLUSTER_ZONE from gke/kubeconfig.sh Jun 26, 2020

@greygoo greygoo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@viccuad

viccuad commented Jun 26, 2020

Copy link
Copy Markdown
Member Author

@prabalsharma, can you have a look? I know it's removing some functionality, but it works nevertheless, and this restores the contract.

@prabalsharma

prabalsharma commented Jun 26, 2020

Copy link
Copy Markdown
Member

@viccuad
GKE kubeconfigs are bit different. They have to be generated using kube clusterreference file. As each generated kubeconfig file has a time stamp you can not use a generated kubeconfig between job as they might get expired in middle of a run.

impossible for the CI to know the name of the cluster you are importing

you get it from pools. eg: https://github.com/SUSE/cf-ci-pools/blob/gke-kube-hosts/gke-kube-hosts/claimed/ci-gke-290bebf661749b86

(nor it needs it anyways)

its needed to generate kubeconfig on each concourse container

@HartS
HartS self-requested a review June 26, 2020 20:45

@HartS HartS left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Echoing what @prabalsharma has said; at the time we introduced the ClusterReference file as way to get a definitely-valid kubeconfig for a GKE cluster, there was no way to get a kubeconfig for GKE cluster which did not expire (or if there was, it wasn't discoverable online)

Last year, a Google Cloud engineer published this blog post which actually describes a way to get this. So if we'd like to move away from a ClusterReference file and instead use configs which don't expire in our pools, that's a possibility.

This would certainly be useful if we might end up running gke operations from an environment where the GKE credentials won't be available, but I'm not convinced we need this work until then; currently, GKE clusters are either accessed via CI (which has the appropriate credentials to get an expiring kubeconfig via the traditional approach with gcloud container clusters get-credentials) or locally when using Catapult (in which case gcloud credentials must be provided in order to perform GKE operations anyway).

@viccuad

viccuad commented Jun 29, 2020

Copy link
Copy Markdown
Member Author

Thanks both for the responses.

I just tried importing and using the same GKE again from this PR, it keeps working fine for what is worth.

Are we sure that expiring creds are an issue?

At minimum, I would like to, without needing to perform changes, import back the GKE kubeconfig that catapult gave me when it created the cluster, without needing to know about the cluster name and the like. So, in that sense I would like to argue at least for changing the gke/deploy.sh script to craft a kubeconfig as needed.

On the blogpost topic, I would prefer to perform kube operations without the need for google cloud credentials, because that way we can chain and interchange CIs in the future with other stakeholders or departments if needed. This means that the cluster is widely accessible just from the kubeconfig (but it would be the case anyway, and we wouldn't need to share google cloud credentials, which would either allow more permissions or need work on closing perms down).

@viccuad

viccuad commented Jun 29, 2020

Copy link
Copy Markdown
Member Author

its needed to generate kubeconfig on each concourse container

Could we just make it random, and track clusters with tags?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants