USHIFT-1590: skip failing invariants in MicroShift #28193

pacevedom · 2023-08-16T15:28:19Z

No description provided.

openshift-ci-robot · 2023-08-16T15:28:22Z

@pacevedom: This pull request references USHIFT-1590 which is a valid jira issue.

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.

pacevedom · 2023-08-17T13:55:28Z

/retest-required

pacevedom · 2023-08-21T09:05:29Z

/cc @ingvagabund @dgoodwin @deads2k

deads2k · 2023-08-24T15:28:00Z

pkg/monitortests/kubeapiserver/disruptionnewapiserver/monitortest.go

+	if !isMicroShift {
+		if err := sampler.TearDownInClusterMonitors(w.adminRESTConfig); err != nil {
+			return err
+		}
+	}


Suggested change

if !isMicroShift {

if err := sampler.TearDownInClusterMonitors(w.adminRESTConfig); err != nil {

return err

}

}

if isMicroShift {

return nil

}

if err := sampler.TearDownInClusterMonitors(w.adminRESTConfig); err != nil {

return err

}

pkg/monitortests/kubeapiserver/disruptionnewapiserver/monitortest.go

deads2k · 2023-08-24T15:29:09Z

pkg/monitortests/network/legacynetworkmonitortests/networking.go

 	} else {
-		infra, err := configClient.ConfigV1().Infrastructures().Get(context.Background(), "cluster", metav1.GetOptions{})
+		isMicroShift, err := exutil.IsMicroShiftCluster(kubeClient)


use early returns to eliminate nesting. This is hard to follow.

deads2k · 2023-08-24T15:34:09Z

pkg/monitortests/network/legacynetworkmonitortests/networking.go

 	} else {
-		infra, err := configClient.ConfigV1().Infrastructures().Get(context.Background(), "cluster", metav1.GetOptions{})
+		isMicroShift, err := exutil.IsMicroShiftCluster(kubeClient)


Perhaps this should become a getPlatformType(restConfig) (platformType, error) to make it more clear. I think this is trying to

fail if a kubeclient cannot be created

if the cluster lacks an infrastructure CRD, return empty, no error

if the cluster has an infrastructure CRD, error if there is no CR instance

if the cluster has a CR instance, return the platform type.

deads2k · 2023-08-24T15:35:12Z

pkg/monitortests/node/kubeletlogcollector/monitortest.go

@@ -30,6 +31,15 @@ func (w *kubeletLogCollector) CollectData(ctx context.Context, storageDir string
 	if err != nil {
 		return nil, nil, err
 	}
+	// MicroShift does not have a proper journal for the node logs api.


!

That's unfortunate. How are users going to collect node logs for kubelet bugs.

cc @rphillips

Its under a different service, the journal belongs to microshift which holds the whole control plane logging output, not just kubelet. All the supportability docs are prepared for it (as well as an sos plugin)

This is the output:

$ oc get --raw "/api/v1/nodes/microshift-dev/proxy/logs" <pre> <a href="README">README</a> <a href="anaconda/">anaconda/</a> <a href="audit/">audit/</a> <a href="btmp">btmp</a> <a href="btmp-20230810">btmp-20230810</a> <a href="chrony/">chrony/</a> <a href="containers/">containers/</a> <a href="crio/">crio/</a> <a href="cron">cron</a> <a href="cron-20230704">cron-20230704</a> <a href="cron-20230810">cron-20230810</a> <a href="cron-20230817">cron-20230817</a> <a href="cron-20230824">cron-20230824</a> <a href="dnf.librepo.log">dnf.librepo.log</a> <a href="dnf.log">dnf.log</a> <a href="dnf.log.1">dnf.log.1</a> <a href="dnf.rpm.log">dnf.rpm.log</a> <a href="firewalld">firewalld</a> <a href="hawkey.log">hawkey.log</a> <a href="hawkey.log-20230704">hawkey.log-20230704</a> <a href="hawkey.log-20230810">hawkey.log-20230810</a> <a href="hawkey.log-20230817">hawkey.log-20230817</a> <a href="hawkey.log-20230824">hawkey.log-20230824</a> <a href="insights-client/">insights-client/</a> <a href="kdump.log">kdump.log</a> <a href="kube-apiserver/">kube-apiserver/</a> <a href="lastlog">lastlog</a> <a href="libvirt/">libvirt/</a> <a href="maillog">maillog</a> <a href="maillog-20230704">maillog-20230704</a> <a href="maillog-20230810">maillog-20230810</a> <a href="maillog-20230817">maillog-20230817</a> <a href="maillog-20230824">maillog-20230824</a> <a href="messages">messages</a> <a href="messages-20230704">messages-20230704</a> <a href="messages-20230810">messages-20230810</a> <a href="messages-20230817">messages-20230817</a> <a href="messages-20230824">messages-20230824</a> <a href="openvswitch/">openvswitch/</a> <a href="ovn/">ovn/</a> <a href="ovn-kubernetes/">ovn-kubernetes/</a> <a href="pods/">pods/</a> <a href="private/">private/</a> <a href="qemu-ga/">qemu-ga/</a> <a href="rhsm/">rhsm/</a> <a href="secure">secure</a> <a href="secure-20230704">secure-20230704</a> <a href="secure-20230810">secure-20230810</a> <a href="secure-20230817">secure-20230817</a> <a href="secure-20230824">secure-20230824</a> <a href="spooler">spooler</a> <a href="spooler-20230704">spooler-20230704</a> <a href="spooler-20230810">spooler-20230810</a> <a href="spooler-20230817">spooler-20230817</a> <a href="spooler-20230824">spooler-20230824</a> <a href="sssd/">sssd/</a> <a href="swtpm/">swtpm/</a> <a href="tallylog">tallylog</a> <a href="wtmp">wtmp</a> </pre>

Test is looking for journal and then filters by the systemd unit for kubelet. In microshift the kubelet is part of microshift systemd unit.

Sounds like you need to fix this code then, not simply skip it.

pkg/monitortests/kubeapiserver/disruptionnewapiserver/monitortest.go

pacevedom · 2023-09-04T21:41:14Z

/retest-required

openshift-ci · 2023-09-05T00:07:05Z

@pacevedom: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-openstack-ovn	`13857ff`	link	false	`/test e2e-openstack-ovn`
ci/prow/e2e-aws-ovn-single-node-upgrade	`13857ff`	link	false	`/test e2e-aws-ovn-single-node-upgrade`
ci/prow/e2e-gcp-ovn-rt-upgrade	`13857ff`	link	false	`/test e2e-gcp-ovn-rt-upgrade`
ci/prow/e2e-metal-ipi-sdn	`13857ff`	link	false	`/test e2e-metal-ipi-sdn`
ci/prow/e2e-aws-ovn-single-node-serial	`13857ff`	link	false	`/test e2e-aws-ovn-single-node-serial`

Full PR test history. Your PR dashboard.

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.

neisw · 2023-09-05T14:10:42Z

/lgtm

pacevedom · 2023-09-05T15:30:27Z

/label acknowledge-critical-fixes-only

deads2k · 2023-09-05T17:41:21Z

pkg/monitortests/network/legacynetworkmonitortests/networking.go

+		return platform, fmt.Errorf("error checking MicroShift cluster: %v", err)
+	}
+	if isMicroShift {
+		return platform, nil


this will be empty, not nil, correct? You sure that's what you want?

Yes, the platform is only needed when deploying on Azure, and thats not the case for MicroShift.

deads2k · 2023-09-05T18:27:06Z

This has made obvious, but will not address shortcomings for addressing CI failures. I'm willing to allow it, but microshift failure resolution will suffer for lack of this.

/approve
/hold

holding for @pacevedom to acknowledge the shortcoming. You may release on acknowledgement without a fix if you wish.

openshift-ci · 2023-09-05T18:27:32Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, neisw, pacevedom

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:

~~pkg/OWNERS~~ [deads2k]

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

pacevedom · 2023-09-06T16:33:30Z

As we already talked about it over Slack, posting here a summary.
The shortcomings account for addressing failures in CI and how to debug them. Since MicroShift does not work/is not deployed like OpenShift there are a few key differences.
In this case, for example, the logs for kubelet and other components are to be found in the journal for microshift itself, not in /var/log/ which is where one of the tests is looking.
To address these limitations, a step in CI is included to run an sos report including microshift specific plugins.
Will work in the future to try to overcome some of these limitations where it makes sense.

/hold cancel because of the reasons above.

pacevedom · 2023-09-06T16:34:00Z

/unhold

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Aug 16, 2023

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 16, 2023

openshift-ci bot requested review from jwforres and mfojtik August 16, 2023 15:38

pacevedom force-pushed the ushift-invariants branch from a821213 to ecdb4fb Compare August 17, 2023 08:45

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 17, 2023

openshift-ci bot requested review from deads2k, dgoodwin and ingvagabund August 21, 2023 09:05

deads2k reviewed Aug 24, 2023

View reviewed changes

neisw reviewed Aug 24, 2023

View reviewed changes

pkg/monitortests/kubeapiserver/disruptionnewapiserver/monitortest.go Show resolved Hide resolved

deads2k reviewed Aug 24, 2023

View reviewed changes

neisw reviewed Aug 25, 2023

View reviewed changes

pkg/monitortests/kubeapiserver/disruptionnewapiserver/monitortest.go Show resolved Hide resolved

pacevedom force-pushed the ushift-invariants branch from 8ba7f64 to 7e65118 Compare August 27, 2023 22:31

openshift-merge-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Aug 31, 2023

pacevedom added 5 commits September 4, 2023 10:14

ushift: skip kubelet journal collection from node

40b1817

ushift: Adapt network invariant to skip infra query

a4723f1

ushift: Adapt apiserver disruption invariant to skip disruption monitor

592974f

ushift: use notSupportedReason in newapiserver disruption monitor

52cf55d

ushift: rework platform retrieval in network monitor tests

13857ff

pacevedom force-pushed the ushift-invariants branch from 7e65118 to 13857ff Compare September 4, 2023 08:15

pacevedom mentioned this pull request Sep 4, 2023

USHIFT-547: Add periodic conformance job openshift/release#42978

Merged

openshift-ci bot assigned neisw Sep 5, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 5, 2023

openshift-ci bot added the acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. label Sep 5, 2023

deads2k reviewed Sep 5, 2023

View reviewed changes

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 5, 2023

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 5, 2023

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 6, 2023

openshift-merge-robot merged commit c3d0116 into openshift:master Sep 6, 2023
17 of 22 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

USHIFT-1590: skip failing invariants in MicroShift #28193

USHIFT-1590: skip failing invariants in MicroShift #28193

pacevedom commented Aug 16, 2023

openshift-ci-robot commented Aug 16, 2023 •

edited by openshift-ci bot

pacevedom commented Aug 17, 2023

pacevedom commented Aug 21, 2023

deads2k Aug 24, 2023

pacevedom Aug 25, 2023

deads2k Aug 24, 2023

deads2k Aug 24, 2023

pacevedom Aug 25, 2023

deads2k Aug 24, 2023

pacevedom Aug 24, 2023

pacevedom Aug 25, 2023

deads2k Sep 5, 2023

pacevedom commented Sep 4, 2023

openshift-ci bot commented Sep 5, 2023

neisw commented Sep 5, 2023

pacevedom commented Sep 5, 2023

deads2k Sep 5, 2023

pacevedom Sep 6, 2023

deads2k commented Sep 5, 2023

openshift-ci bot commented Sep 5, 2023

pacevedom commented Sep 6, 2023

pacevedom commented Sep 6, 2023

USHIFT-1590: skip failing invariants in MicroShift #28193

USHIFT-1590: skip failing invariants in MicroShift #28193

Conversation

pacevedom commented Aug 16, 2023

openshift-ci-robot commented Aug 16, 2023 • edited by openshift-ci bot

pacevedom commented Aug 17, 2023

pacevedom commented Aug 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pacevedom commented Sep 4, 2023

openshift-ci bot commented Sep 5, 2023

neisw commented Sep 5, 2023

pacevedom commented Sep 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deads2k commented Sep 5, 2023

openshift-ci bot commented Sep 5, 2023

pacevedom commented Sep 6, 2023

pacevedom commented Sep 6, 2023

openshift-ci-robot commented Aug 16, 2023 •

edited by openshift-ci bot