Skip to content

Remove e2e-openstack-ccm-install jobs #30952

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
Merged
openshift-merge-robot merged 2 commits into from
Aug 18, 2022
Merged

Remove e2e-openstack-ccm-install jobs #30952

openshift-merge-robot merged 2 commits into from
Aug 18, 2022

Conversation

@MaysaMacedo
Copy link
Contributor

@MaysaMacedo MaysaMacedo commented Aug 1, 2022 •
edited
Loading

As external cloud provider is now the default provider used
for the OCP cluster running on OpenStack, there is no need to
have jobs enabling it as tech preview on master and later releases.
Also, jobs running on previous releases can now be removed.

@MaysaMacedo
Copy link
Contributor Author

MaysaMacedo commented Aug 1, 2022

/hold
let's wait for MCO PR to merge first openshift/machine-config-operator#3270

@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 Aug 1, 2022
@MaysaMacedo MaysaMacedo force-pushed the remove-e2e-openstack-ccm-install branch from ed73944 to fcaf1c8 Compare August 1, 2022 09:38
@MaysaMacedo MaysaMacedo changed the title Remove e2e-openstack-ccm-install jobs for 4.12 release Remove e2e-openstack-ccm-install jobs Aug 1, 2022
@MaysaMacedo MaysaMacedo force-pushed the remove-e2e-openstack-ccm-install branch 2 times, most recently from e422af7 to 5062070 Compare August 8, 2022 11:20
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 8, 2022
@MaysaMacedo MaysaMacedo force-pushed the remove-e2e-openstack-ccm-install branch from 5062070 to 3be68eb Compare August 8, 2022 11:28
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 8, 2022
@MaysaMacedo
Copy link
Contributor Author

MaysaMacedo commented Aug 9, 2022

/hold cancel

@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 Aug 9, 2022
@MaysaMacedo
Copy link
Contributor Author

MaysaMacedo commented Aug 9, 2022

/assign @JoelSpeed @pierreprinetti

@pierreprinetti
Copy link
Member

pierreprinetti commented Aug 9, 2022

Nice, that will give some more room in the CI.

Just a couple nits:

  • please remove changes to priv; those are handled by automation anyway
  • please remove e2e-openstack-techpreview-parallel, e2e-openstack-techpreview-serial and e2e-openstack-ccm-install from shiftstack-ci/shiftstack-shiftstack-ci-main__periodic-4.11.yaml

@MaysaMacedo MaysaMacedo force-pushed the remove-e2e-openstack-ccm-install branch from 3be68eb to aed81ac Compare August 10, 2022 10:27
JoelSpeed
JoelSpeed reviewed Aug 10, 2022
View reviewed changes
Copy link
Contributor

@JoelSpeed JoelSpeed left a comment

Choose a reason for hiding this comment

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

Should we be dropping the CCM tests from older releases that we are still supporting?

...-controller-manager-operator/openshift-cluster-cloud-controller-manager-operator-master.yaml Outdated
Copy link
Contributor

@JoelSpeed JoelSpeed Aug 10, 2022

Choose a reason for hiding this comment

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

Should we rename the test?

Copy link
Contributor Author

@MaysaMacedo MaysaMacedo Aug 10, 2022

Choose a reason for hiding this comment

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

right, I will leave the default of that workflow to be run, which is conformance/parallel plus openstack specific tests.

@pierreprinetti
Copy link
Member

pierreprinetti commented Aug 10, 2022

We can also remove the workflow in ci-operator/step-registry/openshift/e2e/openstack/ccm

@MaysaMacedo
Copy link
Contributor Author

MaysaMacedo commented Aug 10, 2022 •
edited
Loading

ci-operator/step-registry/openshift/e2e/openstack/ccm

I tried doing that. It will fail since we have the jobs on openshift-priv using it. Once this merges and those private jobs are synced, perhaps that can be removed?

@MaysaMacedo MaysaMacedo force-pushed the remove-e2e-openstack-ccm-install branch from aed81ac to 1355f19 Compare August 10, 2022 12:23
@pierreprinetti
Copy link
Member

pierreprinetti commented Aug 10, 2022

ci-operator/step-registry/openshift/e2e/openstack/ccm

I tried doing that. It will fail since we have the jobs on openshift-priv using it. Once this merges and those private jobs are synced, perhaps that can be removed?

Uh-oh right

@pierreprinetti
Copy link
Member

pierreprinetti commented Aug 10, 2022 •
edited
Loading

There are still e2e-openstack-ccm in ci-operator/config/openshift/cluster-cloud-controller-manager-operator. Is that expected?

  • openshift-cluster-cloud-controller-manager-operator-release-4.13.yaml
  • openshift-cluster-cloud-controller-manager-operator-release-4.12.yaml
  • openshift-cluster-cloud-controller-manager-operator-master.yaml

The tests could be called e2e-openstack there now that you've set the default workflow, if I understand Joel's comment.

Wow we had kind of a coverage!

@MaysaMacedo MaysaMacedo force-pushed the remove-e2e-openstack-ccm-install branch from 1355f19 to 3127499 Compare August 11, 2022 07:59
@MaysaMacedo
Copy link
Contributor Author

MaysaMacedo commented Aug 11, 2022

There are still e2e-openstack-ccm in ci-operator/config/openshift/cluster-cloud-controller-manager-operator. Is that expected?

* openshift-cluster-cloud-controller-manager-operator-release-4.13.yaml

* openshift-cluster-cloud-controller-manager-operator-release-4.12.yaml

* openshift-cluster-cloud-controller-manager-operator-master.yaml

The tests could be called e2e-openstack there now that you've set the default workflow, if I understand Joel's comment.

Wow we had kind of a coverage!

done.

@pierreprinetti
Copy link
Member

pierreprinetti commented Aug 11, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 11, 2022
@pierreprinetti
Copy link
Member

pierreprinetti commented Aug 11, 2022

/assign JoelSpeed

@JoelSpeed
Copy link
Contributor

JoelSpeed commented Aug 11, 2022

You've removed CI from the techpreview releases which means if we decide to backport any fixes, we won't be able to test them. I would have expected us to leave the 4.10 and 4.11 branches alone until they are dropped from support. What's the plan?

@MaysaMacedo
Copy link
Contributor Author

MaysaMacedo commented Aug 11, 2022

You've removed CI from the techpreview releases which means if we decide to backport any fixes, we won't be able to test them. I would have expected us to leave the 4.10 and 4.11 branches alone until they are dropped from support. What's the plan?

Since CCM is in tech preview for versions prior to 4.12 and consequently not fully supported by Red Hat, we thought it would be ok to remove those jobs to reduce the amount of jobs we run in the CI. Also, I thought the original idea when adding those jobs was to start testing CCM prior to making it GA. Do you think it's better to remain the presumit jobs? Perhaps those can be left as optional ones

@JoelSpeed
Copy link
Contributor

JoelSpeed commented Aug 11, 2022

My concern is whether we are likely to backport anything to those branches or not. Realistically, there's no harm in leaving the configuration there for presubmits as they will only run if we do decide to backport. If we never create a PR to one of those branches, they will never run

@MaysaMacedo MaysaMacedo force-pushed the remove-e2e-openstack-ccm-install branch from 3127499 to b276e86 Compare August 11, 2022 13:56
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 11, 2022
@MaysaMacedo MaysaMacedo force-pushed the remove-e2e-openstack-ccm-install branch from b276e86 to 6a833df Compare August 11, 2022 14:41
mandre
mandre reviewed Aug 16, 2022
View reviewed changes
Copy link
Member

@mandre mandre 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 cleaning up the jobs that are no longer relevant, hopefully this gives a bit of fresh air to our ci infra.

@MaysaMacedo MaysaMacedo force-pushed the remove-e2e-openstack-ccm-install branch from 6a833df to 5795aa9 Compare August 17, 2022 11:11
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 17, 2022 •
edited
Loading

@MaysaMacedo: The following test 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/rehearse/openshift/cluster-cloud-controller-manager-operator/master/e2e-openstack-ccm 1355f1943ae1386093d6e046861cdbd27ce31475 link unknown /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.

mandre
mandre reviewed Aug 17, 2022
View reviewed changes
Copy link
Member

@mandre mandre 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 openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 17, 2022
@MaysaMacedo
Copy link
Contributor Author

MaysaMacedo commented Aug 18, 2022

@JoelSpeed It seems ready for another look. Thank you!

@MaysaMacedo
Remove openstack ccm jobs
7eb36cd 
As external cloud provider is now the default provider used
for the OCP cluster running on OpenStack, there is no need to
have jobs enabling it as tech preview on master and later releases.
@MaysaMacedo
Increase test suite for CCCMO jobs with OpenStack
66870d2 
As CCM is the default for OpenStack, the test suite that runs on
the CCCMO jobs are now updated to run default tests, which contains
the e2e conformance parallel and openstack tests.
@MaysaMacedo MaysaMacedo force-pushed the remove-e2e-openstack-ccm-install branch from 5795aa9 to 66870d2 Compare August 18, 2022 13:16
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 18, 2022
@JoelSpeed
Copy link
Contributor

JoelSpeed commented Aug 18, 2022

/lgtm
/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 18, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoelSpeed, mandre, MaysaMacedo, pierreprinetti

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 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. labels Aug 18, 2022
@openshift-merge-robot openshift-merge-robot merged commit f6438ba into openshift:master Aug 18, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 18, 2022

@MaysaMacedo: Updated the following 7 configmaps:

  • job-config-master-periodics configmap in namespace ci at cluster app.ci using the following files:
    • key shiftstack-shiftstack-ci-main-periodics.yaml using file ci-operator/jobs/shiftstack/shiftstack-ci/shiftstack-shiftstack-ci-main-periodics.yaml
  • ci-operator-master-configs configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-cloud-provider-openstack-master.yaml using file ci-operator/config/openshift/cloud-provider-openstack/openshift-cloud-provider-openstack-master.yaml
    • key openshift-cluster-cloud-controller-manager-operator-master.yaml using file ci-operator/config/openshift/cluster-cloud-controller-manager-operator/openshift-cluster-cloud-controller-manager-operator-master.yaml
    • key shiftstack-shiftstack-ci-main__periodic-4.11.yaml using file ci-operator/config/shiftstack/shiftstack-ci/shiftstack-shiftstack-ci-main__periodic-4.11.yaml
    • key shiftstack-shiftstack-ci-main__periodic-4.12.yaml using file ci-operator/config/shiftstack/shiftstack-ci/shiftstack-shiftstack-ci-main__periodic-4.12.yaml
  • ci-operator-4.12-configs configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-cloud-provider-openstack-release-4.12.yaml using file ci-operator/config/openshift/cloud-provider-openstack/openshift-cloud-provider-openstack-release-4.12.yaml
    • key openshift-cluster-cloud-controller-manager-operator-release-4.12.yaml using file ci-operator/config/openshift/cluster-cloud-controller-manager-operator/openshift-cluster-cloud-controller-manager-operator-release-4.12.yaml
  • ci-operator-4.13-configs configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-cloud-provider-openstack-release-4.13.yaml using file ci-operator/config/openshift/cloud-provider-openstack/openshift-cloud-provider-openstack-release-4.13.yaml
    • key openshift-cluster-cloud-controller-manager-operator-release-4.13.yaml using file ci-operator/config/openshift/cluster-cloud-controller-manager-operator/openshift-cluster-cloud-controller-manager-operator-release-4.13.yaml
  • job-config-master-presubmits configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-cloud-provider-openstack-master-presubmits.yaml using file ci-operator/jobs/openshift/cloud-provider-openstack/openshift-cloud-provider-openstack-master-presubmits.yaml
    • key openshift-cluster-cloud-controller-manager-operator-master-presubmits.yaml using file ci-operator/jobs/openshift/cluster-cloud-controller-manager-operator/openshift-cluster-cloud-controller-manager-operator-master-presubmits.yaml
  • job-config-4.12 configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-cloud-provider-openstack-release-4.12-presubmits.yaml using file ci-operator/jobs/openshift/cloud-provider-openstack/openshift-cloud-provider-openstack-release-4.12-presubmits.yaml
    • key openshift-cluster-cloud-controller-manager-operator-release-4.12-presubmits.yaml using file ci-operator/jobs/openshift/cluster-cloud-controller-manager-operator/openshift-cluster-cloud-controller-manager-operator-release-4.12-presubmits.yaml
  • job-config-4.13 configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-cloud-provider-openstack-release-4.13-presubmits.yaml using file ci-operator/jobs/openshift/cloud-provider-openstack/openshift-cloud-provider-openstack-release-4.13-presubmits.yaml
    • key openshift-cluster-cloud-controller-manager-operator-release-4.13-presubmits.yaml using file ci-operator/jobs/openshift/cluster-cloud-controller-manager-operator/openshift-cluster-cloud-controller-manager-operator-release-4.13-presubmits.yaml
Details

In response to this:

As external cloud provider is now the default provider used
for the OCP cluster running on OpenStack, there is no need to
have jobs enabling it as tech preview on master and later releases.
Also, jobs running on previous releases can now be removed.

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.

@MaysaMacedo MaysaMacedo deleted the remove-e2e-openstack-ccm-install branch August 18, 2022 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@danilo-gemoli danilo-gemoli Awaiting requested review from danilo-gemoli

@EmilienM EmilienM Awaiting requested review from EmilienM

2 more reviewers

@mandre mandre mandre left review comments

@JoelSpeed JoelSpeed JoelSpeed left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@mandre mandre

@pierreprinetti pierreprinetti

@JoelSpeed JoelSpeed

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.

5 participants

@MaysaMacedo @pierreprinetti @JoelSpeed @mandre @openshift-merge-robot