Skip to content

Conversation

@pierreprinetti
Copy link
Member

@pierreprinetti pierreprinetti commented Jun 17, 2021

This change assumes that all steps in the job will mount SHARED_DIR to
the same mountpoint, as it seems to currently be the case.

@openshift-ci openshift-ci bot requested review from EmilienM and e-tienne June 17, 2021 16:10
This change assumes that all steps in the job will mount SHARED_DIR to
the same mountpoint, as it seems to currently be the case.

That mountpoint is persisted in the cacert property of the clouds.yaml.
@pierreprinetti
Copy link
Member Author

/cc @MaysaMacedo @mdbooth

@mdbooth
Copy link
Contributor

mdbooth commented Jun 17, 2021

This lgtm in principal. Lets see what the test commit shows.

@pierreprinetti pierreprinetti changed the title penstack: Add cacert to clouds.yaml if provided openstack: Add cacert to clouds.yaml if provided Jun 17, 2021
@pierreprinetti
Copy link
Member Author

/test all

Copy link
Member

Choose a reason for hiding this comment

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

I'm trying to understand this change vs its commit message and I don't follow.
From my understanding here, we're switching the e2e-openstack-ipi in the installer project to use mecha cloud instead of managed vexxhost?
I hope not because mecha cloud isn't ready yet (we're having performances issues).

Copy link
Member

Choose a reason for hiding this comment

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

nevermind, I was reading the PR commit message.
Still this cloud is unstable, don't use it yet please.

Copy link
Member Author

Choose a reason for hiding this comment

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

The second commit (that is now removed) was a way to test the change; the change in this PR is just preparatory.

Copy link
Member

Choose a reason for hiding this comment

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

Seems like the commit in question (c61511319841007c9df0b02df7927050bb67ea8b) is still part of the PR. Do you still want to remove it?

Copy link
Member Author

Choose a reason for hiding this comment

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

The second commit (that is NOW removed) ...... 😬

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 18, 2021
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
/hold for CI

@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 Jun 18, 2021
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 18, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 18, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mandre, 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
Copy link
Contributor

openshift-ci bot commented Jun 18, 2021

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

Test name Commit Details Rerun command
ci/rehearse/openshift/installer/master/e2e-openstack-ipi c61511319841007c9df0b02df7927050bb67ea8b link /test pj-rehearse
ci/rehearse/periodic-ci-openshift-release-master-ci-4.9-upgrade-from-stable-4.8-e2e-openstack-upgrade a56c706 link /test pj-rehearse
ci/prow/pj-rehearse a56c706 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.

@pierreprinetti
Copy link
Member Author

@mandre The failure doesn't look related to the change. Do you want to remove the hold?

@mandre
Copy link
Member

mandre commented Jun 18, 2021

/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 Jun 18, 2021
@openshift-merge-robot openshift-merge-robot merged commit 3dc8597 into openshift:master Jun 18, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 18, 2021

@pierreprinetti: Updated the step-registry configmap in namespace ci at cluster app.ci using the following files:

  • key openstack-conf-clouds-commands.sh using file ci-operator/step-registry/openstack/conf/clouds/openstack-conf-clouds-commands.sh
Details

In response to this:

This change assumes that all steps in the job will mount SHARED_DIR to
the same mountpoint, as it seems to currently be the case.

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.

5 participants