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

kubectl image doesn't support (GKE) regional clusters #176

Open
DazWilkin opened this Issue Dec 16, 2017 · 21 comments

Comments

Projects
None yet
10 participants
@DazWilkin
Copy link

DazWilkin commented Dec 16, 2017

Regional Clusters cause challenges for the kubectl image as-is:

  • It's not possible to interact with these clusters using zone flag (they're referred to by region)
  • Current clusters require kubectl beta commands

Near-term -- since it's possible folks will create Regional Clusters and then be flummoxed that this image doesn't work with them -- perhaps flag this documentation to state "The image does not currently work with Regional Clusters"

Longer-term -- the image will need to accommodate a more general ability to refer to cluster locations

@ImJasonH

This comment has been minimized.

Copy link
Member

ImJasonH commented Dec 18, 2017

Let me make sure I have the correct understanding of what you're asking for, before I start modifying code.

The kubectl wrapper calls gcloud container clusters get-credentials with the --zone flag, but this is incompatible with Regional Clusters, in two ways:

  1. It would need to pass the --region flag instead, and
  2. It would need to call gcloud beta container clusters get-credentials to be able to pass the --region flag.

If that's the case, I propose this solution: If the CLOUDSDK_COMPUTE_REGION env var is set, pass it to gcloud beta container clusters get-credentials with --region. If CLOUDSDK_COMPUTE_ZONE is set, pass it as --zone as normal. If both are set, usage+exit. Requisite documentation and examples.

Does that work for you?

@DazWilkin

This comment has been minimized.

Copy link
Author

DazWilkin commented Dec 18, 2017

@ImJasonH -- Thanks for the follow-up.

I think we may be better placed waiting for these features to be GA'd before you make changes.

In the meantime, perhaps a warning "Don't try this with Regional Clusters"?

In my limited experience, the implementation of Regional Clusters adds complexity with how clusters are manifest to the operator: referring to single-zone clusters by zone, multi-zone clusters by primary (?) zone and regional clusters by region smacks of complexity that -- I hope -- will be erased.

@Philmod

This comment has been minimized.

Copy link
Member

Philmod commented Dec 18, 2017

I agree we should wait for the feature to be GA.

@skelterjohn

This comment has been minimized.

Copy link
Collaborator

skelterjohn commented Dec 18, 2017

Since it shouldn't affect any of the GA usage, having support for the beta GKE feature seems fine to me.

@bendory

This comment has been minimized.

Copy link
Member

bendory commented Dec 18, 2017

Is this as simple as (1) being explicit about the version of kubectl that is supported and (2) possibly adding images tagged kubectl:beta, etc. for those who want to live on the edge?

@ImJasonH

This comment has been minimized.

Copy link
Member

ImJasonH commented Dec 18, 2017

We always have the latest version of kubectl that's bundled with the latest gcloud installed (modulo a few-day window if our release is delayed). I don't want to add more complexity than that unless absolutely necessary.

We don't, and I think shouldn't, stop users from invoking kubectl beta ... commands (or gcloud beta ... for that matter).

+1 for documenting this behavior ASAP (PRs welcome if you have verbiage preferences). Beyond that users who know how to use beta features aren't blocked from doing so today.

@Kansuler

This comment has been minimized.

Copy link

Kansuler commented Jan 6, 2018

Do anyone here have a workaround for the kubectl container to get credentials for cluster by region? I see @ImJasonH mentioning something about being able to use beta features, but I can't figure out how to use gcloud beta commands within this container.

@Philmod

This comment has been minimized.

Copy link
Member

Philmod commented Jan 6, 2018

Hey Simon,

You could create your own cloud-builder that installs the beta version of kubectl. You could start with the kubectl Dockerfile.

@DazWilkin

This comment has been minimized.

Copy link
Author

DazWilkin commented Feb 12, 2018

Apologies for my absence. I was going to wait but again wanted to use a regional cluster.

@Kansuler -- an (interim) solution requires a change to the kubectl builder's kubectl.bash not use of a different kubectl. The regional cluster requires gcloud beta and use of --region rather than --zone

#!/bin/bash

# If there is no current context, get one.
if [[ $(kubectl config current-context 2> /dev/null) == "" ]]; then
    cluster=$(gcloud config get-value container/cluster 2> /dev/null)
    region=$(gcloud config get-value compute/region 2> /dev/null)
    project=$(gcloud config get-value core/project 2> /dev/null)

    function var_usage() {
        cat <<EOF
No cluster is set. To set the cluster (and the region where it is found), set the environment variables
  CLOUDSDK_COMPUTE_REGION=<cluster region>
  CLOUDSDK_CONTAINER_CLUSTER=<cluster name>
EOF
        exit 1
    }

    [[ -z "$cluster" ]] && var_usage
    [[ -z "$region" ]] && var_usage

    echo "Running: gcloud beta container clusters get-credentials --project=\"$project\" --region=\"$region\" \"$cluster\""
    gcloud beta container clusters get-credentials --project="$project" --region="$region" "$cluster" || exit
fi

echo "Running: kubectl $@"
kubectl "$@"

The above works. Assuming ${PROJECT} is your project,

  • Clone this repo
  • Use the above for kubectl.bash
  • Build it gcloud container builds submit --config=cloudbuild.yaml . --project=${PROJECT}
  • Reference "gcr.io/${PROJECT}/kubectl" in cloudbuild.yaml to use regional clusters (see below)

NB When referencing this customized kubectl builder you will need to:

  • Explicitly set container/use_v1_api_client in a preceding step
  • replace the ${PROJECT} to its value (the GCP project where you built the builder)
  • or provide a user-defined substitution (in which case you'll need $_PROJECT)
  • or build the builder and use in the same project (in which case you can stick with $PROJECT_ID).

Example:

  - name: "gcr.io/cloud-builders/gcloud"
    args: ["config", "set", "container/use_v1_api_client", "false"]
  - name: "gcr.io/$PROJECT_ID/kubectl"
    args: ["get", "nodes"]
    env: [
      "CLOUDSDK_COMPUTE_REGION=us-west1",
      "CLOUDSDK_CONTAINER_CLUSTER=cluster-01",
    ]
  - name: "gcr.io/$PROJECT_ID/kubectl"
    args: ["get", "deployments"]
    env: [
      "CLOUDSDK_COMPUTE_REGION=us-west1",
      "CLOUDSDK_CONTAINER_CLUSTER=cluster-01",

and:

Step #5: Status: Downloaded newer image for gcr.io/${PROJECT}/kubectl:latest
Step #5: Running: gcloud beta container clusters get-credentials --project="${PROJECT}" --region="${REGION}" "${CLUSTER}"
Step #5: Fetching cluster endpoint and auth data.
Step #5: kubeconfig entry generated for ${CLUSTER}.
Step #5: Running: kubectl get nodes
Step #5: NAME                                        STATUS    ROLES     AGE       VERSION
Step #5: gke-...-default-pool-1a826571-rj18   Ready     <none>    7h        v1.9.2-gke.1
Step #5: gke-...-default-pool-89f8ec7f-1n15   Ready     <none>    8h        v1.9.2-gke.1
Step #5: gke-...-default-pool-c923e078-jg6z   Ready     <none>    6h        v1.9.2-gke.1
Finished Step #5
Starting Step #6
Step #6: Already have image (with digest): gcr.io/${PROJECT}/kubectl
Step #6: Running: kubectl get deployments
Step #6: NAME       DESIRED   CURRENT   UP-TO-DATE   AVAILABLE   AGE
Step #6: depl1      1         1         1            1           8h
Step #6: depl2      0         0         0            0           8h
Step #6: depl3      1         1         1            1           8h
Step #6: depl4      0         0         0            0           8h
Finished Step #6
@Kansuler

This comment has been minimized.

Copy link

Kansuler commented Feb 13, 2018

@DazWilkin Thanks for your guide! I think it will be helpful for others with the same issue, I did create a new container image much like the one you show here. Works well now.

@ImJasonH

This comment has been minimized.

Copy link
Member

ImJasonH commented Feb 13, 2018

@DazWilkin Thanks for writing that up. Care to phrase it in the form of a PR for the official kubectl builder? 😁

@DazWilkin

This comment has been minimized.

Copy link
Author

DazWilkin commented Feb 13, 2018

@ImJasonH -- I'm always willing and thanks for asking, but .... :-)

  1. Regional Clusters do present a challenge today with Cloud SDK because the feature adds "sometimes 'zones'" and 'sometimes 'regions'" complexity that I feel needs to be more consistently addressed by Kubernetes Engine.

  2. I wonder whether the kubectl builder should not be simpler? The builder assumes gcloud which is acceptable if we assume Kubernetes Engine (as this uses gcloud as an auth source) but this makes the builder more complex than necessary and required the creation of a separate kubectl builder because the gcloud (sic.) command changed. I feel this builder should be kubectl only and the user be required to -- if needed -- add a prior gcloud to auth.

Thoughts?

@ImJasonH

This comment has been minimized.

Copy link
Member

ImJasonH commented Feb 13, 2018

@DazWilkin That's entirely fair.

  1. Is the plan for GKE to resolve this complexity themselves, or are we left to handle this ourselves for this builder?

  2. I'm pretty sure this has come up before, at least when we decided to create the kubectl builder in the first place. I believe the thinking was that, since you're using the Google Container Builder, it's not unreasonable to think you might be using Google Kubernetes Engine too, and we wanted to make that as smooth as possible (i.e., one step, metadata auth).

We probably could use a "vanilla" kubectl image that isn't based on gcr.io/cloud-builders/gcloud and doesn't do anything "smart" for GKE. At this point such a downgrade would break basically every user of the builder today. This probably fits well in the community builder repo though.

@DazWilkin

This comment has been minimized.

Copy link
Author

DazWilkin commented Feb 13, 2018

@ImJasonH

  1. Yes, I will confirm. There are some rough edges elsewhere in the CLI. I recall encountering a location flag somewhere else and the response then was that this issue was known but it would be resolved w/ the feature's GA.

  2. Excellent point! Customers do self-manage Kubernetes on GCE as well and their use case would not be covered but -- yes -- the majority may be assumed to be Kubernetes Engine. Perhaps Builder best practice should be to do just one thing, well?

@ImJasonH

This comment has been minimized.

Copy link
Member

ImJasonH commented Feb 23, 2018

FYI the community repo has a PR to add support for regional GKE clusters to the helm builder: https://github.com/GoogleCloudPlatform/cloud-builders-community/pull/32/files

@DazWilkin

This comment has been minimized.

Copy link
Author

DazWilkin commented Feb 23, 2018

I find it messy.

I think a better (best?) solution would have been for cluster names to have been unique by project.

This challenge with referencing is being considered by Kubernetes Engineering.

@victortrac

This comment has been minimized.

Copy link

victortrac commented Feb 23, 2018

I find it messy.

I'm offended! ;)

I made those changes to fix our immediate problems, and it works well for us. It'll be easy enough to fix/update when something changes upstream.

@DazWilkin

This comment has been minimized.

Copy link
Author

DazWilkin commented Feb 23, 2018

@victortrac sorry sorry -- your solution isn't messy... that we're forced to find solutions for clusters that present differently depending on whether they're zonal or regional is what I meant.

stevenvegt added a commit to nedap/irma_api_server that referenced this issue May 15, 2018

@stealthybox

This comment has been minimized.

Copy link

stealthybox commented May 16, 2018

It's worth noting that the --zone flag seems to work for setting regions.
--region may just be an alias:

❯ gcloud beta --project org-gke-us-central-1 container clusters get-credentials gke-cluster1 --zone us-central1
Fetching cluster endpoint and auth data.
kubeconfig entry generated for gke-cluster1.

❯ gcloud beta --project org-gke-us-central-1 container clusters get-credentials gke-cluster1 --region us-central1
Fetching cluster endpoint and auth data.
kubeconfig entry generated for gke-cluster1.

This doesn't address name conflicts across zonal and regional clusters, but this should still be working for most people.

@gsf

This comment has been minimized.

Copy link

gsf commented Jun 6, 2018

Now that Regional Clusters are in GA CLOUDSDK_COMPUTE_ZONE can be set to a region:

steps:
- name: gcr.io/cloud-builders/kubectl
  args:
  - get
  - po
  env:
  - CLOUDSDK_COMPUTE_ZONE=us-central1
  - CLOUDSDK_CONTAINER_CLUSTER=cluster-4
@icco

This comment has been minimized.

Copy link
Member

icco commented Mar 29, 2019

This should be fixed now and support a new _region var

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.