Test that new single replica topology API is taken into account #25812

omertuc · 2021-01-20T14:28:51Z

Tests that deployments have a single replica when openshift/enhancements#555 control plane topology is set to SingleReplica.

This enhancement is yet to be merged as well as the relevant behavior from the various operators, so this PR is in draft mode until then.

test/extended/single_node/topology.go

omertuc · 2021-01-21T13:32:14Z

Thanks for the review, I've refactored the test -

Check only OpenShift namespaces deployments
Use proper client rather than oc
Mechanism to differentiate between infra deployments and control plane deployments

Caveats

Since I'm now using openshift/api lib directly (no oc cli) it directly depends on the Cluster High-availability Mode API enhancements#555 PR and it has to be merged for the ControlPlaneTopology and InfrastructureTopology fields to appear, so this will not compile unless you use go.mod replace github.com/openshift/api to github.com/ravidbro/api at 68bddd86a7957a2c9a13356245ec0a05967b1000

test/extended/single_node/topology.go

markmc · 2021-02-04T16:08:25Z

test/extended/single_node/topology.go

+	}
+}
+
+func isAllowedToFail(deployment appsv1.Deployment) bool {


A comment explaining these special cases would be useful for future reference

Sure. Will add one

Added in 395d469

markmc · 2021-02-04T16:18:44Z

test/extended/single_node/topology.go

+func getNamespaceDeployments(f *e2e.Framework, namespace corev1.Namespace) []appsv1.Deployment {
+	list, err := f.ClientSet.AppsV1().Deployments(namespace.Name).List(context.Background(), metav1.ListOptions{})
+	Expect(err).NotTo(HaveOccurred())
+


Suggested change

return list.Items

You're not doing any filtering here?

Right, no filtering. The loop looks ridiculous because it's leftover after some modifications I did that made it obsolete. Will fix. Thanks

Fixed in 0debdeb

markmc · 2021-02-04T16:25:09Z

test/extended/single_node/topology.go

+	} else {
+		if replicas == 1 {
+			t := GinkgoT()
+			t.Logf("Deployment %s in namespace %s has one replica, consider taking it off the topology allow-list",


Who are we really expecting will react to this message?

It's intended mostly for me. We slowly want to remove operators from that list until it's empty, and it'll be helpful for me to see that message in the logs. It'll be removed once we finally get rid of this list

sttts · 2021-02-09T10:38:17Z

test/extended/single_node/topology.go

+var _ = Describe("[sig-arch] Cluster topology single node tests", func() {
+	f := e2e.NewDefaultFramework("single-node")
+
+	It("Verify that all OpenShift namespaces deployments take control plane topology into account", func() {


Verify that OpenShift components deploy one replica in single node mode

what is "all" ?

what is "all" ?

All deployments defined inside openshift-* namesapces

Re-phrased to "Verify that OpenShift components deploy one replica in SingleReplica topology mode" since it's not called single-node in the API topology enum

romfreiman · 2021-02-09T13:10:39Z

/retest

romfreiman · 2021-02-09T19:37:29Z

/retest

romfreiman · 2021-02-09T20:05:10Z

/retest

romfreiman · 2021-02-09T20:46:36Z

/test e2e-aws-fips

simonpasquier

What about statefulsets? For OpenShift monitoring, we deploy statefulsets for Prometheus, Alertmanager and Thanos ruler (optional).

omertuc · 2021-02-10T11:19:50Z

What about statefulsets? For OpenShift monitoring, we deploy statefulsets for Prometheus, Alertmanager and Thanos ruler (optional).

Right, considered testing those and forgot about that. ~~Will add them to the test.~~

EDIT: I think it's best if I add statefulsets in a future PR, I prefer that this gets merged first.

omertuc · 2021-02-10T13:01:39Z

/retest

omertuc · 2021-02-10T15:35:33Z

/assign @mfojtik

Please approve

romfreiman · 2021-02-10T18:36:50Z

/lgtm

mfojtik · 2021-02-10T18:53:35Z

/approve

@smarterclayton isnt there smarter way to "tag" these tests as single node and only run them in single node job?

openshift-ci-robot · 2021-02-10T18:53:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mfojtik, omertuc, romfreiman

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~test/extended/OWNERS~~ [mfojtik]

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

omertuc · 2021-02-10T19:00:06Z

Why is the merge blocked: Pending — Not mergeable. Job ci/prow/e2e-gcp-builds has not succeeded. ?

e2e-gcp-builds is always_run: false so I'm not sure why is it even running? And why can something be both always_run: false and optional: false?

What can I do about this?

romfreiman · 2021-02-10T19:28:02Z

/test e2e-gcp-builds

romfreiman · 2021-02-10T19:36:04Z

/test e2e-gcp-csi

openshift-bot · 2021-02-10T20:10:15Z

/retest

Please review the full test history for this PR and help us cut down flakes.

omertuc marked this pull request as draft January 20, 2021 14:28

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 20, 2021

openshift-ci-robot requested review from knobunc and smarterclayton January 20, 2021 14:31

omertuc force-pushed the single-node-test branch 2 times, most recently from c69f27d to fba0e91 Compare January 20, 2021 16:43

mrunalp reviewed Jan 20, 2021

View reviewed changes

test/extended/single_node/topology.go Outdated Show resolved Hide resolved

deads2k reviewed Jan 20, 2021

View reviewed changes

test/extended/single_node/topology.go Outdated Show resolved Hide resolved

deads2k reviewed Jan 20, 2021

View reviewed changes

test/extended/single_node/topology.go Outdated Show resolved Hide resolved

romfreiman reviewed Jan 21, 2021

View reviewed changes

test/extended/single_node/topology.go Outdated Show resolved Hide resolved

omertuc force-pushed the single-node-test branch from fba0e91 to 13f42a9 Compare January 21, 2021 13:26

omertuc force-pushed the single-node-test branch from 13f42a9 to e362db3 Compare January 25, 2021 12:20

markmc reviewed Feb 4, 2021

View reviewed changes

omertuc force-pushed the single-node-test branch from 0debdeb to 1fddbdf Compare February 8, 2021 19:33

openshift-ci-robot added the vendor-update Touching vendor dir or related files label Feb 8, 2021

omertuc marked this pull request as ready for review February 8, 2021 19:33

openshift-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 Feb 8, 2021

omertuc force-pushed the single-node-test branch from 1fddbdf to 0debdeb Compare February 8, 2021 19:36

omertuc marked this pull request as draft February 8, 2021 19:36

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 8, 2021

omertuc mentioned this pull request Feb 8, 2021

Upgrade openshift/api to 670ac3fc997c #25869

Merged

omertuc marked this pull request as ready for review February 9, 2021 08:31

openshift-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 Feb 9, 2021

omertuc force-pushed the single-node-test branch from 0debdeb to 41f81c7 Compare February 9, 2021 08:31

sttts reviewed Feb 9, 2021

View reviewed changes

omertuc force-pushed the single-node-test branch from 41f81c7 to 7d189ce Compare February 9, 2021 10:50

Test that new single replica topology API is taken into account

045853f

omertuc force-pushed the single-node-test branch from 7d189ce to 045853f Compare February 9, 2021 21:33

simonpasquier reviewed Feb 10, 2021

View reviewed changes

openshift-ci-robot assigned mfojtik Feb 10, 2021

openshift-ci-robot assigned romfreiman Feb 10, 2021

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 10, 2021

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 10, 2021

openshift-merge-robot merged commit c8eb62d into openshift:master Feb 11, 2021

Test that new single replica topology API is taken into account #25812

Test that new single replica topology API is taken into account #25812

Uh oh!

Conversation

omertuc commented Jan 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

omertuc commented Jan 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

omertuc Feb 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

omertuc Feb 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

omertuc Feb 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

romfreiman commented Feb 9, 2021

Uh oh!

romfreiman commented Feb 9, 2021

Uh oh!

romfreiman commented Feb 9, 2021

Uh oh!

romfreiman commented Feb 9, 2021

Uh oh!

simonpasquier left a comment

Choose a reason for hiding this comment

Uh oh!

omertuc commented Feb 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

omertuc commented Feb 10, 2021

Uh oh!

omertuc commented Feb 10, 2021

Uh oh!

romfreiman commented Feb 10, 2021

Uh oh!

mfojtik commented Feb 10, 2021

Uh oh!

openshift-ci-robot commented Feb 10, 2021

Uh oh!

omertuc commented Feb 10, 2021

Uh oh!

romfreiman commented Feb 10, 2021

Uh oh!

romfreiman commented Feb 10, 2021

Uh oh!

openshift-bot commented Feb 10, 2021

Uh oh!

Reviewers

Assignees

omertuc commented Jan 20, 2021 •

edited

Loading

omertuc commented Jan 21, 2021 •

edited

Loading

omertuc Feb 4, 2021 •

edited

Loading

omertuc Feb 9, 2021 •

edited

Loading

omertuc Feb 9, 2021 •

edited

Loading

omertuc commented Feb 10, 2021 •

edited

Loading