Initial behavior examples for conformance #85960

johnbelamaric · 2019-12-05T18:35:02Z

What type of PR is this?
/kind cleanup

What this PR does / why we need it:
Updates to the conformance tooling as defined in this KEP.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:
This is a work-in-progress, the spec.yaml and readiness-gates.yaml are the furthest along.

The initial files were generated with kubetestgen and from there I re-oraganized and tweaked them. Files in this format will eventually become the standard description of what conformant Kubernetes is, and the tests will just validate these behaviors. Behaviors may be flagged with a Provisional status, to indicate that they are desired behaviors but we do not yet have a test that covers them. This will be helpful during the transition, although it likely will be needed for quite some time as we backfill tests.

The KEP describes linking the tests and the behaviors throught a tests.yaml file, but instead I think it will be easier just to add a section to the conformance test meta-data:

 /*
  Testname: Create and Delete a Pod with a Single Container and Default Values
  Description: This test creates a pod with a single container in and otherwise default PodSpec values, then deletes it. The Pod must be scheduled to a node and the container enter Running state. The Pod must be deleted and the terminationGracePeriod default of 30 seconds respected.
  Behaviors:
   - pods/basic-create
   - pods/basic-delete
*/

The test runner then can emit the covered behaviors, simplifying producing a report of whether a given cluster is conformant. All non-provisional behaviors must be hit in the test run; this avoids "missing" tests going undetected.

We should consider how to divide the meta-data between the YAML and the test comment. For example, "version" in the YAML would apply to what Kubernetes version expects that behavior. Version in the test comment may refer to when the test was added, if we believe we need it at all.

The expectation is that we can define the list of behaviors up front, much more rapidly than we can define and write the tests themselves. We must therefore decide whether clusters that pass all existing tests but demonstrably do not meet Provisional behaviors are conformant. I would suggest they are not.

We still need the manual review process for any tests that claims to be hitting conformance behaviors. The reviewers must be careful to ensure that any implicit behaviors that are relied upon by a conformance test are also conformant. I don't see any way to automate that. However, the pool of reviewers/approvers for those tests can be much wider than the pool of reviewers/approvers for the behaviors themselves.

Does this PR introduce a user-facing change?:

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

KEP

johnbelamaric · 2019-12-05T18:41:43Z

/hold

johnbelamaric · 2019-12-05T18:42:44Z

/cc @spiffxp @johnSchnake @Jefftree @timothysc @hh

Jefftree · 2019-12-27T16:29:42Z

test/conformance/behaviors/pods/affinity.yaml

@@ -0,0 +1,205 @@
+- suite: pods/affinity
+  level: ""


Was this field generated? It doesn't seem to be in our yaml spec.

johnbelamaric · 2020-01-28T23:11:05Z

/retitle Initial behavior examples for conformance

johnbelamaric · 2020-01-28T23:12:03Z

/priority important-soon

Jefftree

I can't comment on the actual behaviors, but left a few notes for file format and structure.

Jefftree · 2020-01-29T00:14:38Z

test/conformance/kubetestgen/types.go

@@ -35,5 +35,7 @@ type Behavior struct {
 	APIObject   string `json:"apiObject,omitempty"`
 	APIField    string `json:"apiField,omitempty"`
 	APIType     string `json:"apiType,omitempty"`
+	Version     string `json:"version,omitempty"`
+	Status      string `json:"status,omitempty"`


What is this field for? It doesn't seem to be used.

The idea was to be able to have provisional behaviors that we know should be part of conformance but that we also know are not covered yet. In order to be conforming, a cluster needs to meet the required behaviors. That is, we want to be able to define the behaviors before the tests are ready.

I can add in a value for this for all the behaviors in this PR.

ack. I'm worried this might introduce human error where we forget to remove this even after writing a test. Personally, I feel that the CI tooling should be able to determine this when generating the behavior <> test mapping. At the same time though, if it's defined here, coverage should be extremely easy to compute.

Jefftree · 2020-01-29T00:15:17Z

test/conformance/kubetestgen/types.go

@@ -35,5 +35,7 @@ type Behavior struct {
 	APIObject   string `json:"apiObject,omitempty"`
 	APIField    string `json:"apiField,omitempty"`
 	APIType     string `json:"apiType,omitempty"`
+	Version     string `json:"version,omitempty"`


Should we prepend v1.18 (or whatever the version should be) to all the behaviors we're adding?

Hmm. Good question. Most of these behaviors have been around for a long, long time. But once we transition to the behavior model, then it will really only apply to those newer versions. I had put in v1.14 for the readiness gates; that's when it went GA. But it would be easier to just make them all v1.18, since no earlier cluster would be subject to these rules.

Should I just remove API* fields? They were intended to provide context / help with regeneration when using kubetestgen. But at least for now we're not really re-generating things, but instead just using them for bootstrapping. Different PR though.

I think it's helpful in certain situations when we're just enumerating over one field (eg service type)...although we should probably be more explicit in specifying which fields are mandatory vs optional.

Different PR, I agree.

I ended up removing version and status for now, we can deal with that later. I also simplified the initial lists of behaviors.

Jefftree · 2020-01-29T00:15:47Z

test/conformance/behaviors/sig-node/pod-readiness-gates.yaml

@@ -0,0 +1,16 @@
+- suite: pods/readinessGates
+  level: ""


This can be removed

Jefftree · 2020-01-29T00:17:55Z

test/conformance/behaviors/sig-network/service-spec.yaml

@@ -0,0 +1,45 @@
+suite: services/spec


Let's be consistent in the file and suite name. Either services-spec.yaml or service/spec for suite name

Jefftree · 2020-01-29T00:18:12Z

test/conformance/behaviors/sig-node/pod-readiness-gates.yaml

@@ -0,0 +1,16 @@
+- suite: pods/readinessGates


Same comment about consistency in file and suite name.

Jefftree · 2020-01-29T00:18:19Z

test/conformance/behaviors/sig-node/pod-spec.yaml

@@ -0,0 +1,103 @@
+suite: pods/spec


Same comment about consistency in file and suite name.

Organizes the behaviors directory based on SIG, and provides a few example behavior descriptions for Pods and Services to start. Note that these are very incomplete lists of behaviors at this point.

k8s-ci-robot · 2020-01-31T19:26:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johnbelamaric

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:

~~test/conformance/behaviors/OWNERS~~ [johnbelamaric]

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

johnbelamaric · 2020-01-31T19:27:40Z

/unhold

Jefftree · 2020-01-31T19:33:52Z

/lgtm

fejta-bot · 2020-02-01T00:02:05Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.