Add behavior <-> conformance test linkage example for pods #89716
Conversation
k8s-ci-robot
added
release-note-none
Jefftree
force-pushed
the
bdd-initial-link
branch
from
edd908c
to
c956d88
Compare
k8s-ci-robot
added
area/conformance
|
/cc |
Jefftree
force-pushed
the
bdd-initial-link
branch
from
c956d88
to
c4bd15b
Compare
k8s-ci-robot
added
kind/cleanup
|
/assign @johnbelamaric |
|
/override pull-kubernetes-node-e2e-containerd |
|
@spiffxp: Overrode contexts on behalf of spiffxp: pull-kubernetes-node-e2e-containerd In response to this:
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. |
| description: When hostIPC is set to false, the Pod MUST NOT use the host's inter-process | ||
| communication namespace. | ||
| - id: pod/spec/basic-update | ||
| decription: Create a Pod with a unique label. Query for the Pod with the label as selector MUST be successful. Update the pod to change the value of the Label. Query for the Pod with the new value for the label MUST be successful. |
There was a problem hiding this comment.
This isn't a useful behavior description. It should be much smaller in scope. In fact this is represents several behaviors: creating a pod with a label, querying by label, and changing a label. Those all fall into "basic machinery" of the API server. As I recall we need to have a discussion with sig api-machinery about a way to generically test these behaviors across all resources. This would apply to anything utilizing only metadata.
For this particular test, which already exists, we can add those few behaviors. Something like:
- When creating a Pod you MUST be able to set a label, and querying using Pods using a namespace and label selector for that label MUST return all Pods with that label in that namespace and no other Pods. [ yes this is in fact multiple behaviors but so tightly bound I am ok with it being one ]
- A patch request MUST be able to update a label on a Pod. Querying for the Pod with the new label in that namespace and no other Pods.
I do not think a "basic-update" is a useful behavior, at least not for podspec which has very few mutable fields. Instead, we should identify the specific fields that are mutable within a podspec, and create behaviors describing what happens when you mutate those fields. So, for example, tolerations on a pod are mutable. We should have behaviors describing how taints & tolerations work - there are some conformance tests around that already. The behaviors should capture the expression of the change, not just that it happened in the API. That is, it should capture the data plane changes.
There was a problem hiding this comment.
Not sure on patch vs put here. Are they both possible? do they both behave the same way?
There was a problem hiding this comment.
I don't have an opinion on patch vs put.
For the initial load of behaviours, I'd like to get a list through that we can start soon rather that wait for every.
There was a problem hiding this comment.
I do agree that the basic update behavior isn't useful since it applies to the "basic machinery". For now though, I'd like to avoid changing the tests when we're in the process of just porting over the behaviors to the new linkage machinery.
I divided this into two behaviors, but I agree this test will need to be revisited in the future.
| - id: pod/spec/basic-update | ||
| decription: Create a Pod with a unique label. Query for the Pod with the label as selector MUST be successful. Update the pod to change the value of the Label. Query for the Pod with the new value for the label MUST be successful. | ||
| - id: pod/spec/container/resources | ||
| description: Create a Pod with CPU and Memory request and limits. Pod status MUST have QOSClass set to PodQOSGuaranteed. |
There was a problem hiding this comment.
Ok, but does the test cover any of the specific logic around scheduling, eviction, etc. for guaranteed pods? If so we need behaviors for all of those. If not that's OK I suppose; those will show up later in sig-scheduling behaviors.
There was a problem hiding this comment.
This only checks for the corresponding status tags. I think the logic you mentioned should be deferred to sig-scheduling.
|
/priority important-soon |
k8s-ci-robot
added
priority/important-soon
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
@johnbelamaric do you still desire edits? |
Jefftree
force-pushed
the
bdd-initial-link
branch
from
c4bd15b
to
c7216b9
Compare
k8s-ci-robot
removed
the
lgtm
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/approve |
|
/test pull-kubernetes-e2e-gce |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Jefftree, johnbelamaric, spiffxp 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 |
k8s-ci-robot
added
the
approved
|
/retest |
|
@Jefftree: The following test failed, say
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. |
|
/retest Review the full test history for this PR. Silence the bot with an |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Link two conformance tests to corresponding pod spec behaviors.
There are more conformance tests in the
e2e/common/pods.gofile, but I don't think they belong in the pod spec section.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: