Skip to content

Conversation

@stbenjam
Copy link
Member

@stbenjam stbenjam commented Jun 3, 2021

CI and nightly releases are not part of the official Red Hat
Cincinnati graphs. This commit removes the channel property 1,
which will result in the NoChannel condition 2, but will keep the
CVO from attempting to find its current version in the official
Cincinnati 3. That in turn should keep the CVO from throwing a new,
critical alert 4, which will keep it from running afoul of recent
update e2e logic that forbids critical alerts 5.

See this related PR 6 which does this for other platforms IPI
install step.

Co-authored-by: W. Trevor King [email protected]

CI and nightly releases are not part of the official Red Hat
Cincinnati graphs.  This commit removes the channel property [1],
which will result in the NoChannel condition [2], but will keep the
CVO from attempting to find its current version in the official
Cincinnati [3].  That in turn should keep the CVO from throwing a new,
critical alert [4], which will keep it from running afoul of recent
update e2e logic that forbids critical alerts [5].

See this related PR [6] which does this for other platforms IPI
install step.

[1]: https://github.com/openshift/installer/blob/4eca2efd615f8abd65f576721e2410b19f0d40d0/data/data/manifests/bootkube/cvo-overrides.yaml.template#L8
[2]: https://github.com/openshift/cluster-version-operator/blob/fa452c2d270f1f989f3868ef97ae8cf825713583/docs/user/status.md#nochannel
[3]: https://bugzilla.redhat.com/show_bug.cgi?id=1827378#c4
[4]: openshift/cluster-version-operator#357
[5]: openshift/origin#24786
[6]: openshift/release#8631

Co-authored-by: W. Trevor King <[email protected]>
@openshift-ci openshift-ci bot requested review from russellb and vrutkovs June 3, 2021 14:26
Copy link
Contributor

@wking wking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci
Copy link

openshift-ci bot commented Jun 3, 2021

@wking: changing LGTM is restricted to collaborators

Details

In response to this:

/lgtm

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.

@sadasu
Copy link
Contributor

sadasu commented Jun 3, 2021

/retest

@stbenjam
Copy link
Member Author

stbenjam commented Jun 3, 2021

I wonder if we're starting to hit disk space issues? I'm not entirely sure what this error means:

        s: "expected pod \"downwardapi-volume-fb468687-c67d-47c5-96c5-78cfdb00f266\" success: pod \"downwardapi-volume-fb468687-c67d-47c5-96c5-78cfdb00f266\" failed with status: {Phase:Failed Conditions:[] Message:Pod The node had condition: [DiskPressure].  Reason:Evicted NominatedNodeName: HostIP: PodIP: PodIPs:[] StartTime:2021-06-03 22:18:44 +0000 UTC InitContainerStatuses:[] ContainerStatuses:[] QOSClass: EphemeralContainerStatuses:[]}",

/test e2e-metal-ipi

@hardys
Copy link

hardys commented Jun 8, 2021

I wonder if we're starting to hit disk space issues? I'm not entirely sure what this error means:

        s: "expected pod \"downwardapi-volume-fb468687-c67d-47c5-96c5-78cfdb00f266\" success: pod \"downwardapi-volume-fb468687-c67d-47c5-96c5-78cfdb00f266\" failed with status: {Phase:Failed Conditions:[] Message:Pod The node had condition: [DiskPressure].  Reason:Evicted NominatedNodeName: HostIP: PodIP: PodIPs:[] StartTime:2021-06-03 22:18:44 +0000 UTC InitContainerStatuses:[] ContainerStatuses:[] QOSClass: EphemeralContainerStatuses:[]}",

/test e2e-metal-ipi

Similar issues reported for the -upgrade job via https://bugzilla.redhat.com/show_bug.cgi?id=1968754 - I was wondering if it was specific to upgrade but it sounds like probably not?

We could increase the disk size ref #1257 but it'd be good to understand why that's become required now

@hardys
Copy link

hardys commented Jul 5, 2021

/retest


# For CI and nightly releases we need to remove the channel property to
# prevent critical alerts.
sed -i '/^ channel:/d' "${assets_dir}/manifests/cvo-overrides.yaml"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be conditional, since we do support a release type of "ga" in addition to the "ci" and "nightly" options?

https://github.com/openshift-metal3/dev-scripts/blob/master/common.sh#L117

@hardys
Copy link

hardys commented Sep 22, 2021

/test e2e-metal-ipi

@hardys
Copy link

hardys commented Sep 23, 2021

/approve

I still suspect we want to make this conditional so it's not applied for GA builds (@stbenjam @wking can you confirm?)

However since this is impacting CI right now, and AFAIK not many folks are testing GA builds via these scripts, we could address that as a followup

@openshift-ci
Copy link

openshift-ci bot commented Sep 23, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hardys

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 23, 2021
@ardaguclu
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 23, 2021
@openshift-merge-robot openshift-merge-robot merged commit 17461ae into openshift-metal3:master Sep 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants