kubetest2: Infer the provider and zones from the kops cluster

[justinsb]

This means we don't need to pass these flags explicitly.

[justinsb]

It looks like we weren't running cloudprovider specific tests, AFAICT.

/assign @rifelpet

[rifelpet]

You're right, we are missing cloud provider tests as well as a few others that I'm working to enable (see this comment, though my fix didn't seem to help)

I'm not sure how I feel about importing these dependencies into kubetest2, it feels like kubetest2 should be more standalone and operate agnostic of any kops version. That said, we do need a way of knowing which cloud provider the cluster is using. I brought this up in the kubetest2 repo: kubernetes-sigs/kubetest2#87 (comment) and one suggestion was to use a well-known file that the deployer creates and the tester reads. Perhaps we could do that? just a json or yaml file that contains the cloud provider and zones and nothing else.

Alternatively we could do a more naive unmarshal of kops get cluster -o yaml, or better yet add support for -o jsonpath (I'm sure this would be significantly more work though)

Thoughts?

[justinsb]

Good point on the imports @rifelpet ! I'm deliberately only using the models (e.g. I'm not using the vfs functions to read direct from the state store). We could reimplement the models (just the fields we need); we could also make the apis into the own go module, which would help with the volume of dependencies.

We need so few fields I can easily reimplement, but over time we'll need more (e.g. I have a test for etcd status) and it'll become more tedious. That said, these are part of our contract, so it isn't like changing them is common/easy.

[rifelpet]

I'm wondering if we should replace these the relative path to the local vendor directory, that way we dont need to keep two go.mod files in sync anytime we bump these.

[rifelpet]

Just tried this locally and it doesn't work. replace k8s.io/api => ../../vendor/k8s.io/api will fail because go expects the go.mod and go.sum files to exist at the location, but vendoring modules explicitly does not include them.

Looks like we'll just need to keep these in sync going forward

[rifelpet]

retrying now that we're testing with k8s 1.21

/retest

[rifelpet]

/test pull-kops-e2e-k8s-gce

[johngmyers]

/retest

[rifelpet]

The AWS test failures all seem to be permissions related:

event for exec-volume-test-preprovisionedpv-cwxl: {attachdetach-controller } FailedAttachVolume: AttachVolume.Attach failed for volume "aws-zj78w" : error attaching EBS volume "vol-00aaae1fd7f2591ed" to instance "i-081097ce79d433274": "UnauthorizedOperation: You are not authorized to perform this operation.

That error is from KCM which uses the master IAM role which has AttachVolume permissions on volumes tagged with KubernetesCluster=<clustername>:

kops/pkg/model/iam/iam_builder.go

Lines 896 to 914 in af353d1

	&Statement{
	Effect: StatementEffectAllow,
	Action: stringorslice.Of(
	"ec2:AttachVolume", // aws.go
	"ec2:AuthorizeSecurityGroupIngress", // aws.go
	"ec2:CreateRoute", // aws.go
	"ec2:DeleteRoute", // aws.go
	"ec2:DeleteSecurityGroup", // aws.go
	"ec2:DeleteVolume", // aws.go
	"ec2:DetachVolume", // aws.go
	"ec2:RevokeSecurityGroupIngress", // aws.go
	),
	Resource: resource,
	Condition: Condition{
	"StringEquals": map[string]string{
	"ec2:ResourceTag/KubernetesCluster": clusterName,
	},
	},
	},

Viewing the testgrid history of the job, scrolling far enough to the right reveals that these same tests were passing before the kubetest2 migration around April 2nd. Here is the final kubetest1 job with passing tests.

Comparing the ginkgo test args, these were being provided before and are no longer being provided now:
--gce-region=ca-central-1 --gce-multizone=false --cluster-tag=e2e-de198719ee-ff1eb.test-cncf-aws.k8s.io --repo-root=. --disable-log-dump=true

I'm wondering if the e2e tests use the --cluster-tag to tag the EBS volumes it creates, and without the flag the master IAM role doesn't have permission to attach it. I'll try adding those flags next.

[rifelpet]

Yep I'm guessing thats the case :)

https://github.com/kubernetes/kubernetes/blob/9fd42639a07f44097ec733f2ef2a780895505c08/test/e2e/framework/providers/aws/aws.go#L104-L124

the GCE failure is because we're not passing the gcp project to the tester and kops binary that it shells out to. The tester also needs to provide the project flag to the e2e test suite. I'll address both issues shortly

[rifelpet]

/test pull-kops-e2e-k8s-gce

[rifelpet]

Ok the additional tags have reduced us down to one failure in the AWS jobs, and I believe I have a fix for the current issue in the GCE job

/test pull-kops-e2e-k8s-gce

[rifelpet]

The failing AWS test is blocked by kubernetes/kubernetes#101443

Regarding the GCE test failures, since I recently added the presubmit job with kubetest2 I'll have to use the periodic job as a reference point to determine how to proceed. The GCE presubmit is optional though, so I suppose this PR doesn't necessarily need to be blocked by getting it to pass.

[rifelpet]

/test pull-kops-e2e-k8s-gce

[rifelpet]

/retest

I skipped the failing AWS KMS test until it is fixed upstream, so the aws jobs should pass now.

Since the gce job is new and optional we can work on addressing its failures independently of this PR

[k8s-ci-robot]

@justinsb: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-kops-e2e-k8s-gce	8d5e46c	link	/test pull-kops-e2e-k8s-gce

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

[hakman]

/skip all

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hakman

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:

• OWNERS [hakman]

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