Skip to content

Conversation

rifelpet
Copy link
Member

@rifelpet rifelpet commented Oct 8, 2020

ref: #9598
This creates a new go module for the e2e code and the kubetest2 skeleton.
Most of the kubetest2 code was copied from sigs.k8s.io/kubetest2/kubetest2-gke.

Currently only building (make gcs-publish-ci) is in place.
I used test-infra/scenarios/kubernetes_e2e.py as reference, removing environment and make variables that are no longer needed.

Notes:

  • This is a new go module so that anyone importing kops does not end up with transitive dependencies used only by e2e testing. (gopls users may need to update to 0.5.1 for multi-module workspace support)

  • I elected not to vendor the dependencies as suggested by Justin

  • I envision kops-specific e2e tests could live in tests/e2e/ as well as Cluster and IG template files from which we can launch clusters. (we'll need to templatize them to adjust the cluster name, region and other fields which are specific to each job run)

  • For presubmits we'll compile kubetest2-kops with the same commit being tested, and install kubetest2 with the version defined in go.mod. Because periodics don't clone the kops repo we'll need a way to use a specific kubetest2-kops binary. A few ideas for this:

    • Build and publish the kubetest2 and kubetest2-kops binaries on postsubmit along with the other kops artifacts. This may get into a chicken/egg problem - periodics would need to pull the kops artifacts down before running any kubetest2 commands (I was originally assuming kubetest2-kops would download the artifacts)
    • Build and promote an image that we'll use for prow jobs - a postsubmit job will build & push the image and we'll manually bump the image tags in the test-infra job definitions whenever we want periodics to use a new version of kubetest2-kops.
    • Have the periodic jobs clone the kops repo using extra_refs and build kubetest2 and kubetest2-kops the same way that presubmits would.

    We'll need to keep in mind release branching and which versions of kubetest2-kops we use to test which release branches of kops.

Next Steps:

  • Get our CI / verify jobs to play nicely with the new go module (i was envisioning we could eventually move tools.go into its own go module too)
  • Setup an optional presubmit prow job that performs the instructions below
  • Implement Up() and Down() for AWS with just kops create cluster and kops delete cluster. This will require determining all the environment variables and file paths that are in place for the prow jobs, including setting up the boskos client.

Instructions:

cd tests/e2e
go install sigs.k8s.io/kubetest2
go install ./kubetest2-kops
# runs `make gcs-publish-ci` and exits
kubetest2 kops --build --stage-location=gs://foobar/ --kops-root=../../

Help text:

kubetest2 kops --help
Usage:
  kubetest2 kops [Flags] [DeployerFlags] -- [TesterArgs]

Flags:
      --artifacts string   top-level directory to put artifacts under for each kubetest2 run, defaulting to "${ARTIFACTS:-./_artifacts}". If using the ginkgo tester, this must be an absolute path. (default "src/k8s.io/kops/tests/e2e/_artifacts")
      --build              build kubernetes
      --down               tear down the test cluster
  -h, --help               display help
      --run-id string      unique identifier for a kubetest2 run (default "78e3e602-c079-403f-a0cd-8319b7993cc2")
      --test string        test type to run, if unset no tests will run
      --up                 provision the test cluster

DeployerFlags(kops):
      --add_dir_header                   If true, adds the file directory to the header of the log messages
      --alsologtostderr                  log to standard error as well as files
      --kops-root string                 Path to root of the kops repo. Used with --build.
      --log_backtrace_at traceLocation   when logging hits line file:N, emit a stack trace (default :0)
      --log_dir string                   If non-empty, write log files in this directory
      --log_file string                  If non-empty, use this log file
      --log_file_max_size uint           Defines the maximum size a log file can grow to. Unit is megabytes. If the value is 0, the maximum file size is unlimited. (default 1800)
      --logtostderr                      log to standard error instead of files (default true)
      --skip_headers                     If true, avoid header prefixes in the log messages
      --skip_log_headers                 If true, avoid headers when opening log files
      --stage-location string            Storage location for kops artifacts. Only gs:// paths are supported.
      --stderrthreshold severity         logs at or above this threshold go to stderr (default 2)
  -v, --v Level                          number for the log level verbosity
      --vmodule moduleSpec               comma-separated list of pattern=N settings for file-filtered logging

This creates a new go module for the e2e code and the kubetest2 skeleton.
Most of the kubetest2 code was copied from sigs.k8s.io/kubetest2/kubetest2-gke.

Currently only building (`make gcs-publish-ci`) is in place.
I used test-infra/scenarios/kubernetes_e2e.py as reference, removing env and make variables that are no longer needed.

Instructions:
```
cd tests/e2e
go install sigs.k8s.io/kubetest2
go install ./kubetest2-kops
kubetest2 kops -v 9 --build --stage-location=gs://foobar/ --kops-root=../../  # runs make gcs-publish-ci and exits
```
@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 8, 2020
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 8, 2020
@rifelpet rifelpet force-pushed the kubetest2 branch 2 times, most recently from 1f5d236 to 9703c64 Compare October 9, 2020 00:32
@rifelpet
Copy link
Member Author

rifelpet commented Oct 9, 2020

/cc @justinsb by request
/cc @olemarkus since you mentioned being interested in e2e testing at the hacktoberfest meeting

@k8s-ci-robot
Copy link
Contributor

@rifelpet: GitHub didn't allow me to request PR reviews from the following users: in, the, request, interested, meeting, testing, at, hacktoberfest, by, being, e2e, since, you.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @justinsb by request
/cc @olemarkus since you mentioned being interested in e2e testing at the hacktoberfest meeting

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.

@hakman
Copy link
Member

hakman commented Oct 9, 2020

I am interested also :)
/cc @hakman

@k8s-ci-robot k8s-ci-robot requested a review from hakman October 9, 2020 04:26
@rifelpet rifelpet changed the title WIP Initial kubetest2 structure for e2e testing Initial kubetest2 structure for e2e testing Oct 10, 2020
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 10, 2020
@olemarkus
Copy link
Member

This is a great start!

I wonder if we should think of doing update tests too. Spin up a cluste with spec A, apply spec B, run some e2e. To test that e.g ELB -> NLB migration ends up in a working cluster. This test suite probably don't have to run that often, maybe just pre-release.

func (d *deployer) initialize() error {
if d.commonOptions.ShouldBuild() {
if err := d.verifyBuildFlags(); err != nil {
return fmt.Errorf("init failed to check build flags: %s", err)
Copy link
Member

Choose a reason for hiding this comment

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

%w or %v I think

func bindFlags(d *deployer) *pflag.FlagSet {
flags, err := gpflag.Parse(d)
if err != nil {
klog.Fatalf("unable to generate flags from deployer")
Copy link
Member

Choose a reason for hiding this comment

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

Nit: please include the error in the message

@justinsb
Copy link
Member

A great start - let's merge and iterate!

/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 20, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: justinsb, rifelpet

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

@k8s-ci-robot k8s-ci-robot merged commit 3ba214d into kubernetes:master Oct 20, 2020
@k8s-ci-robot k8s-ci-robot added this to the v1.19 milestone Oct 20, 2020
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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. 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.

5 participants