Skip to content

machine-config-operator: fix tests in 4.6 and 4.7 to be in sync with master#12615

Merged
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
sinnykumari:master
Oct 12, 2020
Merged

machine-config-operator: fix tests in 4.6 and 4.7 to be in sync with master#12615
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
sinnykumari:master

Conversation

@sinnykumari
Copy link
Copy Markdown
Contributor

@sinnykumari sinnykumari commented Oct 9, 2020

After 4.7 branch cut, it seems some of the tests which were
optional has become REQUIRED.
Make e2e-metal-ipi test optional,
make aws-proxy and cluster-bootimages always_run to false

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 9, 2020
@sinnykumari
Copy link
Copy Markdown
Contributor Author

sinnykumari commented Oct 9, 2020

/cc @kikisdeliveryservice @yuqi-zhang

@sinnykumari sinnykumari changed the title machine-config-operator: fix tests in 4.6 and 4.7 to be in sync with master machine-config-operator: fix tests in 4.6 and 4.7 to be in sync with master and add back OKD tests Oct 9, 2020
@sinnykumari
Copy link
Copy Markdown
Contributor Author

sinnykumari commented Oct 9, 2020

/cc @vrutkovs

@vrutkovs
Copy link
Copy Markdown
Contributor

vrutkovs commented Oct 9, 2020

I think it would be best to copy ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-master__okd.yaml to -release-4.6__okd.yaml and autogenerate tests (updating always_run and optional)

@ashcrow ashcrow requested review from runcom and removed request for ashcrow October 9, 2020 14:15
yuqi-zhang
yuqi-zhang approved these changes Oct 9, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@yuqi-zhang yuqi-zhang left a comment

Choose a reason for hiding this comment

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

Changes look fine to me compared to master, looking at the history the ci-bot updated these fields 3-4 months ago last. I presume we never used the 4.6 branch tests until the cut so we never hit it?

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 9, 2020
@openshift-bot
Copy link
Copy Markdown
Contributor

openshift-bot commented Oct 9, 2020

/retest

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

1 similar comment
@openshift-bot
Copy link
Copy Markdown
Contributor

openshift-bot commented Oct 9, 2020

/retest

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

@vrutkovs
Copy link
Copy Markdown
Contributor

vrutkovs commented Oct 9, 2020

/hold

needs manifest update

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 9, 2020
@kikisdeliveryservice
Copy link
Copy Markdown
Contributor

kikisdeliveryservice commented Oct 9, 2020

Changes look fine to me compared to master, looking at the history the ci-bot updated these fields 3-4 months ago last. I presume we never used the 4.6 branch tests until the cut so we never hit it?

yeah 4.6 tests would have been master tests until 4.6 was cut.

kikisdeliveryservice
kikisdeliveryservice suggested changes Oct 9, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@kikisdeliveryservice kikisdeliveryservice left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Going by memory when I added a test last:

If you want to add tests (the okd tests) you need to update:

https://github.com/openshift/release/blob/master/ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-master.yaml

https://github.com/openshift/release/blob/master/ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-4.6.yaml

https://github.com/openshift/release/blob/master/ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-4.7.yaml

then run make update, then go in and manually fix the always_run & optional stuff

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 9, 2020
@vrutkovs vrutkovs mentioned this pull request Oct 12, 2020
Merged
@sinnykumari
machine-config-operator: fix tests in 4.6 and 4.7 to be in sync with …
238621a 
…master

After 4.7 branch cut, it seems some of the tests which were
optional has become REQUIRED.
Make e2e-metal-ipi test optional,
make aws-proxy and cluster-bootimages always_run to false
@sinnykumari
Copy link
Copy Markdown
Contributor Author

sinnykumari commented Oct 12, 2020

Removed OKD related changes since it has handled in #12683

@sinnykumari sinnykumari changed the title machine-config-operator: fix tests in 4.6 and 4.7 to be in sync with master and add back OKD tests machine-config-operator: fix tests in 4.6 and 4.7 to be in sync with master Oct 12, 2020
@sinnykumari
Copy link
Copy Markdown
Contributor Author

sinnykumari commented Oct 12, 2020

removing hold as we are not handling okd tests here now and hence manifests update is not required now.
/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 12, 2020
@runcom
Copy link
Copy Markdown
Member

runcom commented Oct 12, 2020

/approve
/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 12, 2020
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

openshift-ci-robot commented Oct 12, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: runcom, sinnykumari, yuqi-zhang

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-merge-robot openshift-merge-robot merged commit a7e608c into openshift:master Oct 12, 2020
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

openshift-ci-robot commented Oct 12, 2020

@sinnykumari: Updated the following 4 configmaps:

  • job-config-4.7 configmap in namespace ci at cluster api.ci using the following files:
    • key openshift-machine-config-operator-release-4.7-presubmits.yaml using file ci-operator/jobs/openshift/machine-config-operator/openshift-machine-config-operator-release-4.7-presubmits.yaml
  • job-config-4.6 configmap in namespace ci at cluster api.ci using the following files:
    • key openshift-machine-config-operator-release-4.6-presubmits.yaml using file ci-operator/jobs/openshift/machine-config-operator/openshift-machine-config-operator-release-4.6-presubmits.yaml
  • job-config-4.6 configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-machine-config-operator-release-4.6-presubmits.yaml using file ci-operator/jobs/openshift/machine-config-operator/openshift-machine-config-operator-release-4.6-presubmits.yaml
  • job-config-4.7 configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-machine-config-operator-release-4.7-presubmits.yaml using file ci-operator/jobs/openshift/machine-config-operator/openshift-machine-config-operator-release-4.7-presubmits.yaml
Details

In response to this:

After 4.7 branch cut, it seems some of the tests which were
optional has become REQUIRED.
Make e2e-metal-ipi test optional,
make aws-proxy and cluster-bootimages always_run to false

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ericavonb ericavonb Awaiting requested review from ericavonb

@vrutkovs vrutkovs Awaiting requested review from vrutkovs

@runcom runcom Awaiting requested review from runcom

2 more reviewers

@kikisdeliveryservice kikisdeliveryservice kikisdeliveryservice requested changes

@yuqi-zhang yuqi-zhang yuqi-zhang approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@runcom runcom

@kikisdeliveryservice kikisdeliveryservice

@yuqi-zhang yuqi-zhang

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.

8 participants

@sinnykumari @vrutkovs @openshift-bot @kikisdeliveryservice @runcom @openshift-ci-robot @yuqi-zhang @openshift-merge-robot