Skip to content

Conversation

@detiber
Copy link

@detiber detiber commented Nov 13, 2020

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 13, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign elmiko after the PR has been reviewed.
You can assign the PR to them by writing /assign @elmiko in a comment when ready.

The full list of commands accepted by this bot can be found 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-robot
Copy link
Contributor

Hi @detiber. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@openshift-ci-robot openshift-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Nov 13, 2020
go.mod Outdated

replace sigs.k8s.io/cluster-api-provider-azure => github.com/openshift/cluster-api-provider-azure v0.1.0-alpha.3.0.20201016155852-4090a6970205

replace github.com/openshift/api => github.com/detiber/api v0.0.0-20201113192824-c19de3594400
Copy link
Author

Choose a reason for hiding this comment

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

Relies on changes in openshift/api#784

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 21, 2020
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 24, 2020
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 5, 2020
Copy link
Contributor

@elmiko elmiko left a comment

Choose a reason for hiding this comment

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

this looks reasonable to me. i have not had a chance to run it yet, although the binary build and unit tests worked. i had a minor suggestion about the CCO repo, but it's not a blocker.

i think we should put a hold here just to make sure we don't do anything before the api changes merge.
/lgtm
/hold until openshift/api#784 has merged

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

elmiko commented Jan 12, 2021

one more comment, this should be rebased as some of the baremetal stuff has been removed since it was first posted.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 12, 2021

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

Test name Commit Details Rerun command
ci/prow/yaml-lint 4e41837 link /test yaml-lint
ci/prow/unit 4e41837 link /test unit
ci/prow/e2e-aws-operator 4e41837 link /test e2e-aws-operator
ci/prow/e2e-gcp-operator 4e41837 link /test e2e-gcp-operator
ci/prow/e2e-aws 4e41837 link /test e2e-aws
ci/prow/e2e-azure 4e41837 link /test e2e-azure
ci/prow/e2e-gcp 4e41837 link /test e2e-gcp
ci/prow/generate 4e41837 link /test generate
ci/prow/e2e-vsphere 4e41837 link /test e2e-vsphere
ci/prow/goimports 4e41837 link /test goimports
ci/prow/e2e-vsphere-operator 4e41837 link /test e2e-vsphere-operator
ci/prow/e2e-azure-operator 4e41837 link /test e2e-azure-operator
ci/prow/e2e-metal-ipi 4e41837 link /test e2e-metal-ipi
ci/prow/images 4e41837 link /test images
ci/prow/govet 4e41837 link /test govet
ci/prow/e2e-aws-workers-rhel7 4e41837 link /test e2e-aws-workers-rhel7
ci/prow/golint 4e41837 link /test golint
ci/prow/e2e-libvirt 4e41837 link /test e2e-libvirt
ci/prow/e2e-aws-upgrade 4e41837 link /test e2e-aws-upgrade

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.

@openshift-ci-robot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 13, 2021
go.mod Outdated

replace sigs.k8s.io/cluster-api-provider-azure => github.com/openshift/cluster-api-provider-azure v0.1.0-alpha.3.0.20201130182513-88b90230f2a4

replace github.com/openshift/api => github.com/detiber/api v0.0.0-20210113181726-19e9a251beff
Copy link
Author

Choose a reason for hiding this comment

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

Temporary workaround until openshift/api#784 is merged

Copy link
Contributor

Choose a reason for hiding this comment

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

+1, that makes sense for now

@detiber
Copy link
Author

detiber commented Jan 13, 2021

Rebased, I'm having difficulty running the tests locally, they don't like running outside of docker (running hack/ci-test.sh directly due to not having the kubebuilder binaries installed where it wants them) and running in docker doesn't seem happy.

Not sure how to reconcile at the moment, but may try digging in further a little be latter on down the line.

@elmiko
Copy link
Contributor

elmiko commented Jan 13, 2021

Rebased, I'm having difficulty running the tests locally, they don't like running outside of docker (running hack/ci-test.sh directly due to not having the kubebuilder binaries installed where it wants them) and running in docker doesn't seem happy.

yeah, i apologize for this but it's a known deficiency currently. i will spend some time to see if i can provide a good recipe for running these.

Not sure how to reconcile at the moment, but may try digging in further a little be latter on down the line.

i am going to take another look as well to see if we can't make this easier for you. this is a known issue

@elmiko
Copy link
Contributor

elmiko commented Jan 13, 2021

@detiber i just saw that this pr got posted #789 , i haven't tested it too much yet but just wanted to share.

@detiber
Copy link
Author

detiber commented Jan 14, 2021

Updated, did some squashing, added the commit from #789 to enable local testing

@elmiko
Copy link
Contributor

elmiko commented Jan 14, 2021

those changes worked for me, i was able to build the binary with make build and run the unit tests with make test. bonus, all the tests passed.

@elmiko
Copy link
Contributor

elmiko commented Feb 19, 2021

just wanted to leave an update here, i am still working towards live testing of the equinix metal provider. i have access to the equinix platform and have been working through the upi installation process for openshift. my next step will be to add in the equinix specific provider bits and run some manual testing followed by attempting to run our e2e tests.

@openshift-bot
Copy link
Contributor

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 27, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 27, 2021

@detiber: PR needs rebase.

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.

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 27, 2021
@openshift-bot
Copy link
Contributor

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci openshift-ci bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jun 26, 2021
@openshift-bot
Copy link
Contributor

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

@openshift-ci openshift-ci bot closed this Jul 26, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 26, 2021

@openshift-bot: Closed this PR.

Details

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

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.

@displague
Copy link

/reopen

@displague
Copy link

/remove-lifecycle rotten

@openshift-ci openshift-ci bot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Aug 24, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 24, 2021

@displague: You can't reopen an issue/PR unless you authored it or you are a collaborator.

Details

In response to this:

/reopen

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

do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants