Skip to content

Conversation

@omertuc
Copy link
Contributor

@omertuc omertuc commented Jan 11, 2021

Based on #15187

Copy link
Contributor

@kikisdeliveryservice kikisdeliveryservice left a comment

Choose a reason for hiding this comment

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

@kikisdeliveryservice
Copy link
Contributor

We don't manually generate those test files anymore... Basically you update the master.yaml file I linked to above then run:

$ make update

then fix the fields as i suggested..

@omertuc omertuc force-pushed the omer/machine-config-operator-single-node branch from ca9422d to 512d401 Compare January 11, 2021 21:35
@omertuc
Copy link
Contributor Author

omertuc commented Jan 11, 2021

We don't manually generate those test files anymore... Basically you update the master.yaml file I linked to above then run:

$ make update

then fix the fields as i suggested..

I didn't manually generate, I put:

- as: e2e-aws-single-node
  commands: TEST_SUITE=openshift/conformance/parallel run-tests
  openshift_installer:
    cluster_profile: aws

In the config file you linked, generated, then deleted the above snippet.

The reason I deleted it is because:

- name: CLUSTER_VARIANT
- value: single-node

are manually added to the job, and they get destroyed when you try to re-generate.

If you know how I can specify the CLUSTER_VARIANT template parameter in the config file, it'll be much better, otherwise I can't think of any other solution other than removing the config that was used to generate so the snippet above doesn't get overridden.

optional and always_run that get manually modified get special treatment in that regard. The regeneration knows to skip them. The CLUSTER_VARIANT snippet however, does not.

@omertuc omertuc force-pushed the omer/machine-config-operator-single-node branch from 512d401 to c8e622a Compare January 11, 2021 21:51
@kikisdeliveryservice
Copy link
Contributor

thanks @omertuc I'm just going to ask around and get this double checked before we merge. thanks for explaining.

@omertuc
Copy link
Contributor Author

omertuc commented Jan 11, 2021

thanks @omertuc I'm just going to ask around and get this double checked before we merge. thanks for explaining.

Great, keep me posted.

@openshift-merge-robot
Copy link
Contributor

@omertuc: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/pj-rehearse c8e622ab4e6a387a6db509eff634c8d910ded403 link /test pj-rehearse

Full PR test history. Your PR dashboard.

Details

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.

@kikisdeliveryservice
Copy link
Contributor

Will look into this later today and if I can't figure out a better solution won't block merging.

In the meantime

/retest

@kikisdeliveryservice
Copy link
Contributor

kikisdeliveryservice commented Jan 19, 2021

The selection seems to be coming from :
https://docs.ci.openshift.org/docs/how-tos/contributing-openshift-release/#tolerated-changes-to-generated-jobs

@stevekuznetsov are there any major issues implementing this job as @omertuc has done ? Or we'd need to tolerate that change which seems like a bigger thing. My one concern would be - will this job get picked up for future branches too (unsure how those are generated - do they just copy master?- but prowgen was removed here)?

@kikisdeliveryservice
Copy link
Contributor

just adding a hold until this is updated

/hold

@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 Jan 19, 2021
@omertuc omertuc force-pushed the omer/machine-config-operator-single-node branch 2 times, most recently from 00a48a4 to 009f65a Compare January 27, 2021 00:05
@kikisdeliveryservice
Copy link
Contributor

kikisdeliveryservice commented Jan 27, 2021

@stevekuznetsov could you PTAL?

This seems correct to me, but should pj-reherse pass (I'm not totally sure?)

@omertuc
Copy link
Contributor Author

omertuc commented Jan 27, 2021

@stevekuznetsov could you PTAL?

This seems correct to me, but should pj-reherse pass (I'm not totally sure?)

always run false, optional true. We don't expect it to pass at this point. There are still a lot of fixes needed in the various operators for this to work.

@kikisdeliveryservice
Copy link
Contributor

@stevekuznetsov could you PTAL?
This seems correct to me, but should pj-reherse pass (I'm not totally sure?)

always run false, optional true. We don't expect it to pass at this point. There are still a lot of fixes needed in the various operators for this to work.

yes i see that 😄

but I'd like his review to double check that these tests are now fine.

Copy link
Contributor

@kikisdeliveryservice kikisdeliveryservice left a comment

Choose a reason for hiding this comment

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

spoke with stevek and this is good to go.

/lgtm

@openshift-ci-robot openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 27, 2021
@kikisdeliveryservice
Copy link
Contributor

/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 Jan 27, 2021
@openshift-bot
Copy link
Contributor

/retest

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

2 similar comments
@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

18 similar comments
@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@omertuc omertuc force-pushed the omer/machine-config-operator-single-node branch from 009f65a to a7981d1 Compare January 28, 2021 16:01
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jan 28, 2021
@omertuc
Copy link
Contributor Author

omertuc commented Jan 28, 2021

@omertuc looks like there's an error in

prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_release/14756/pull-ci-openshift-release-master-ci-operator-config-metadata/1354525915652034560

That needs to be resolved?

Right, fixed now, should be okay

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 28, 2021

@omertuc: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/rehearse/openshift/machine-config-operator/master/e2e-aws-single-node a7981d1 link /test pj-rehearse
ci/prow/pj-rehearse a7981d1 link /test pj-rehearse

Full PR test history. Your PR dashboard.

Details

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.

Copy link
Contributor

@kikisdeliveryservice kikisdeliveryservice left a comment

Choose a reason for hiding this comment

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

thanks for the update !

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 28, 2021
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kikisdeliveryservice, omertuc

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-bot
Copy link
Contributor

/retest

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

@openshift-merge-robot openshift-merge-robot merged commit 79d22c3 into openshift:master Jan 28, 2021
@openshift-ci-robot
Copy link
Contributor

@omertuc: Updated the following 3 configmaps:

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

In response to this:

Based on #15187

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

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