Conformance: Add StatefulSet tests. #60336
Conversation
enisoc
added
sig/apps
k8s-ci-robot
added
release-note
| @@ -901,21 +913,29 @@ var _ = SIGDescribe("StatefulSet", func() { | |||
| framework.DeleteAllStatefulSets(c, ns) | |||
| }) | |||
|
|
|||
| // This can't be Conformance yet because it depends on a default | |||
There was a problem hiding this comment.
This should never be conformance. We should consider removing it. The same is true for all of the application specific tests that follow it. We haven't removed them because they have been improved to the point that they provide a signal, but we shouldn't require them for API conformance.
There was a problem hiding this comment.
Is it because they are application-specific, or because they rely on dynamic volume provisioning, which is an optional capability?
There was a problem hiding this comment.
Ken's comment was originally on the ZooKeeper example, and he was referring only to the app-specific tests. I rewrote the comments on those to explain that they are excluded for being app-specific.
After the squash and force push, GitHub has mistakenly placed his comment on a totally different line (that happens to have the same contents as the line he previously commented on).
The "should provide basic identity" test was never in question for being app-specific. It is only excluded because it relies on dynamic provisioning and a default storage class, as indicated in the comment.
test/e2e/apps/statefulset.go
Outdated
| @@ -242,7 +248,8 @@ var _ = SIGDescribe("StatefulSet", func() { | |||
| sst.WaitForRunningAndReady(*ss.Spec.Replicas, ss) | |||
| }) | |||
|
|
|||
| It("should perform rolling updates and roll backs of template modifications", func() { | |||
| // This is ok for Conformance because it doesn't use any PVCs. | |||
| framework.ConformanceIt("should perform rolling updates and roll backs of template modifications", func() { | |||
There was a problem hiding this comment.
You need to add the appropriate conformance comment. See the README in the conformance dir. The same goes for all selected conformance tests.
test/e2e/apps/statefulset.go
Outdated
| @@ -566,7 +574,8 @@ var _ = SIGDescribe("StatefulSet", func() { | |||
|
|
|||
| }) | |||
|
|
|||
| It("should implement legacy replacement when the update strategy is OnDelete", func() { | |||
| // This is ok for Conformance because it doesn't use any PVCs. | |||
| framework.ConformanceIt("should implement legacy replacement when the update strategy is OnDelete", func() { | |||
There was a problem hiding this comment.
Don't make this conformance. It's not clear that OnDelete will continue to be a supported update strategy after extensions/v1beta1 is removed.
enisoc
force-pushed
the
statefulset-conformance
branch
from
ca49717 to
9b266e9
Compare
enisoc
force-pushed
the
statefulset-conformance
branch
from
9b266e9 to
1e82d13
Compare
|
@kow3ns Comments addressed. PTAL. |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
LGTM from a conformance perspective. It looks like the "This can't be Conformance yet because..." comments still need some rewording. |
|
@bgrant0607 GitHub moved @kow3ns' comment around, which made it look unresolved. In reality, all his comments have been resolved. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bgrant0607, enisoc, kow3ns 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
|
Automatic merge from submit-queue (batch tested with PRs 59674, 60059, 60220, 58916, 60336). If you want to cherry-pick this change to another branch, please follow the instructions here. |
Mark StatefulSet tests as Conformance where possible. I've excluded those that depend on a dynamic provisioner and a default storage class (i.e. those that use PVC), because I don't think those things are required for Conformance at this time.
@kow3ns @jagosan Please correct me if I'm wrong.
Part of #54256