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

Add a task for creating a GKE cluster for e2e tests 🧪 #436

Merged
merged 1 commit into from
Jul 24, 2020

Conversation

bobcatfish
Copy link
Contributor

Changes

As part of #373 I'm continuing to create Tasks that we can use to run
tests for items in the catalog. This Task creates a cluster within a GKE
project and is based on the way we create clusters for the existing end
to end tests we have in Tekton, which are themselves based on Knative,
when are THEMselves based on k8s. These tests all use a tool called
kubetest, which is responsible for invoking boskos (see #408) and
creating a GKE cluster
(https://github.com/kubernetes/test-infra/tree/master/kubetest).

This Task attempts to create a cluster in the same way, which based on the
output in the end to end logs seems to consist of creating the cluster
and setting up a firewall for it.

I'm not sure if this belongs in the catalog itself since it is specific
to our own end to end tests and isn't as generic as other catalog tests,
so if we want to move this into the test directory that seems fine, but
I also think it could be an okay addition to the catalog on its own (esp
if other k8s based projects that test against GKE want to use it.)

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

  • Includes docs (if user facing)
  • Commit messages follow commit message best practices
  • Yaml file complies with yamllint rules.
  • Complies with Catalog Orgainization TEP, see example. Note An issue has been filed to automate this validation
    • File path follows <kind>/<name>/<version>/name.yaml

    • Has README.md at <kind>/<name>/<version>/README.md

    • Has mandatory metadata.labels - app.kubernetes.io/version the same as the <version> of the resource

    • Has mandatory metadata.annotations tekton.dev/pipelines.minVersion

    • mandatory spec.description follows the convention

        ```
      
        spec:
          description: >-
            one line summary of the resource
      
            Paragraph(s) to describe the resource.
        ```
      

See the contribution guide
for more details.


@tekton-robot tekton-robot requested review from a user and vdemeester July 22, 2020 20:59
@tekton-robot tekton-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 22, 2020
@bobcatfish
Copy link
Contributor Author

@ixdy this might be relevant to your interests! I'm trying to replicate more kubetest functionality in Tekton Tasks. Also if you happen to know of anything else that is done to the clusters kubetest creates that I've missed plz lemme know.

Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 23, 2020
@bobcatfish
Copy link
Contributor Author

/test pull-tekton-catalog-integration-tests

@@ -0,0 +1,43 @@
# GKE End to End Cluster Create
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason the name and tags all include "e2e" and "end to end"? This seems much more broadly useful than the end to end tests, even though that's what you're using it for.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't sure what else to call it - i thought it would be doing more e2e specific stuff, but I guess the only thing is the firewall? Maybe that's a reasonable firewall to start a cluster off with? I'm happy to just call it create GKE cluster!

# Create a network and a new cluster
gcloud compute networks create $UNIQUE_NAME --project $(params.project-name) --subnet-mode=auto
gcloud beta container clusters create \
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to use beta here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i don't know! thats what kubetest was doing but ill remove it and if that still works then 👍

# Configure gcloud to use the provided project and service account
gcloud auth activate-service-account --key-file=$(workspaces.gcp-service-account.path)/$(params.private-key-path)
gcloud config set project $(params.project-name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this call really required? you pass --project on every command below anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh yeah good point

plus this command always gives me this weird warning about the project not existing which is just not true :D

--allow=tcp:22,tcp:80,tcp:8080,tcp:30000-32767,udp:30000-32767 \
--target-tags=$INSTANCE_TAG
printf $UNIQUE_NAME > /tekton/results/cluster-name
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: newline

spec:
resources:
requests:
storage: 5Gi
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure it matters, but the kubeconfig probably won't be 5 gigs :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but how do you know

--allow=tcp:22,tcp:80,tcp:8080,tcp:30000-32767,udp:30000-32767 \
--target-tags=$INSTANCE_TAG
printf $UNIQUE_NAME > /tekton/results/cluster-name
Copy link
Contributor

Choose a reason for hiding this comment

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

Oooh one other one - the cluster name by itself isn't actually that useful - you generally need the region/zone to access it as well. Would that make sense as a result too? Or I guess we maybe don't need it since it's a param.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

whatever you think is best! it feels a bit weird to replicated something that is passed in as a param but if you think it's worth it, ill do it!

(maybe one day we'll have some kind of "cluster info" result type that will have all of this info?)

@bobcatfish
Copy link
Contributor Author

ps @dlorenc do you wanna be an OWNER of this Task? no pressure :D

As part of tektoncd#373 I'm continuing to create Tasks that we can use to run
tests for items in the catalog. This Task creates a cluster within a GKE
project and is based on the way we create clusters for the existing end
to end tests we have in Tekton, which are themselves based on Knative,
when are THEMselves based on k8s. These tests all use a tool called
kubetest, which is responsible for invoking boskos (see tektoncd#408) and
creating a GKE cluster
(https://github.com/kubernetes/test-infra/tree/master/kubetest).

This Task attempts to create a cluster in the same way, which based on the
output in the end to end logs seems to consist of creating the cluster
and setting up a firewall for it.

I'm not sure if this belongs in the catalog itself since it is specific
to our own end to end tests and isn't as generic as other catalog tests,
so if we want to move this into the test directory that seems fine, but
I also think it could be an okay addition to the catalog on its own (esp
if other k8s based projects that test against GKE want to use it.)
@tekton-robot tekton-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 23, 2020
@bobcatfish
Copy link
Contributor Author

Ready for another look @dlorenc !

@dlorenc
Copy link
Contributor

dlorenc commented Jul 24, 2020

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 24, 2020
Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

/meow

@tekton-robot
Copy link

@vdemeester: cat image

In response to this:

/meow

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.

@tekton-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vdemeester

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 24, 2020
@tekton-robot tekton-robot merged commit 5cf3952 into tektoncd:master Jul 24, 2020
bobcatfish added a commit to bobcatfish/catalog that referenced this pull request Aug 5, 2020
As part of tektoncd#373 I'm continuing to create Tasks that we can use to run
tests for items in the catalog.

The goal of the 2 Tasks in this commit is to install Tekton Pipelines into a
cluster that was created using the Task added in tektoncd#436.

There are a couple of options different from what I did in this commit
which I'd like people's opinions on:
1. I went to a lot of trouble to make it so that Pipelines can be
   installed with just a kubectl command, specifically creating a user
   with cluster admin priviledges. If instead I used a container with
   gcloud, I could use the same approach as other GKE specific Tasks,
   but I wanted to make the install Task not tied to GKE. I also had
   to give the prow-account more priviledges in all of our boskos
   projects for this to work :grimace:. So my question is: is this worth
   it, or am I better off just creating a GKE specific Task for
   installing Tekton Pipelines that uses gcloud to auth?
2. If we DO want a super generic Task, how useful is this Task that just
   applies a Tekton yaml? Would we be better off with a generic kubectl
   task that I could feed the exact arguments for installing Tekton
   Pipelines?

The other confusion I ran into here (which I am going to follow up with
@sbwsg about!) is around service accounts vs. secrets. The GKE Tasks
I've made so far rely on secrets provided via workspaces, but I feel
like it's not clear if it makes sense to go that route, or to rely on
service accounts. In the meantime this approach is at least consistent
with tektoncd#436.
bobcatfish added a commit to bobcatfish/catalog that referenced this pull request Aug 5, 2020
As part of tektoncd#373 I'm continuing to create Tasks that we can use to run
tests for items in the catalog.

The goal of the 2 Tasks in this commit is to install Tekton Pipelines into a
cluster that was created using the Task added in tektoncd#436.

There are a couple of options different from what I did in this commit
which I'd like people's opinions on:
1. I went to a lot of trouble to make it so that Pipelines can be
   installed with just a kubectl command, specifically creating a user
   with cluster admin priviledges. If instead I used a container with
   gcloud, I could use the same approach as other GKE specific Tasks,
   but I wanted to make the install Task not tied to GKE. I also had
   to give the prow-account more priviledges in all of our boskos
   projects for this to work :grimace:. So my question is: is this worth
   it, or am I better off just creating a GKE specific Task for
   installing Tekton Pipelines that uses gcloud to auth?
2. If we DO want a super generic Task, how useful is this Task that just
   applies a Tekton yaml? Would we be better off with a generic kubectl
   task that I could feed the exact arguments for installing Tekton
   Pipelines?

The other confusion I ran into here (which I am going to follow up with
@sbwsg about!) is around service accounts vs. secrets. The GKE Tasks
I've made so far rely on secrets provided via workspaces, but I feel
like it's not clear if it makes sense to go that route, or to rely on
service accounts. In the meantime this approach is at least consistent
with tektoncd#436.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants