Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

USHIFT-351: Microshift: skip sig-devex tests #27392

Merged
merged 1 commit into from
Oct 5, 2022
Merged

USHIFT-351: Microshift: skip sig-devex tests #27392

merged 1 commit into from
Oct 5, 2022

Conversation

pacevedom
Copy link
Contributor

No description provided.

@pacevedom pacevedom changed the title USHIFT-341: Microshift: skip sig-devex tests USHIFT-351: Microshift: skip sig-devex tests Sep 2, 2022
@pacevedom
Copy link
Contributor Author

/retest-required

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 8, 2022
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 12, 2022
@pacevedom
Copy link
Contributor Author

Ready for review.
/assign @ingvagabund

@pacevedom
Copy link
Contributor Author

/retest-required

@pacevedom
Copy link
Contributor Author

/hold
Need to get of microshift checks

@openshift-ci 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 23, 2022
@pacevedom
Copy link
Contributor Author

/unhold
Using apigroups now.

@openshift-ci 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 23, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 23, 2022 •
edited
Loading

@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-gcp-ovn-rt-upgrade bfdc3d8 link false /test e2e-gcp-ovn-rt-upgrade
ci/prow/e2e-aws-ovn-single-node-upgrade bfdc3d8 link false /test e2e-aws-ovn-single-node-upgrade
ci/prow/e2e-aws-ovn-single-node-serial bfdc3d8 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.

ingvagabund
ingvagabund reviewed Sep 29, 2022
View reviewed changes
test/extended/controller_manager/pull_secret.go
@@ -129,7 +129,7 @@ var _ = g.Describe("[sig-devex][Feature:OpenShiftControllerManager]", func() {
}
})

g.It("TestDockercfgTokenDeletedController", func() {
g.It("TestDockercfgTokenDeletedController [apigroup:image.openshift.io]", func() {
Copy link
Member

Choose a reason for hiding this comment

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

Where does the image apigroup requirement come from? waitForServiceAccountPullSecret?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, waitForServiceAccountPullSecret expects a secret to be provisioned in order to make use of the image API.

Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to say by which component is the secret provisioned?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be the openshift controller manager.

Copy link
Member

@ingvagabund ingvagabund Oct 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

TestAutomaticCreationOfPullSecrets follows the same workflow with the default sa name. Checking for presence of image/cluster object. Which implies dependency on the image apigroup.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That test is updated too: https://github.com/openshift/origin/pull/27392/files#diff-e548b9ac6cbe0bee7cc02da7c4b6ced6eb44801339dfea05220e4285b7eec558R63
Or did I misunderstand you?

Copy link
Member

Choose a reason for hiding this comment

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

I was just justifying why adding image api group into TestDockercfgTokenDeletedController is required. The test itself does not give any indication why the image group is needed. Though, following TestAutomaticCreationOfPullSecrets gives some idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh ok, didnt follow you. Thanks!

@ingvagabund
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 4, 2022
@pacevedom
Copy link
Contributor Author

/retest-required

soltysh
soltysh approved these changes Oct 4, 2022
View reviewed changes
Copy link
Contributor

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 4, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ingvagabund, pacevedom, soltysh

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 /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 Oct 4, 2022
@openshift-merge-robot openshift-merge-robot merged commit 703e3d1 into openshift:master Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ingvagabund ingvagabund ingvagabund left review comments

@soltysh soltysh soltysh approved these changes

@deads2k deads2k Awaiting requested review from deads2k

@knobunc knobunc Awaiting requested review from knobunc

Assignees

@ingvagabund ingvagabund

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pacevedom @ingvagabund @soltysh @openshift-merge-robot