Skip to content

[OCPCLOUD-1187] Include Azure support to external CCM selection#1077

Merged
openshift-merge-robot merged 4 commits intoopenshift:masterfrom
Danil-Grigorev:external-azure
Jul 2, 2021
Merged

[OCPCLOUD-1187] Include Azure support to external CCM selection#1077
openshift-merge-robot merged 4 commits intoopenshift:masterfrom
Danil-Grigorev:external-azure

Conversation

@Danil-Grigorev
Copy link
Copy Markdown

@Danil-Grigorev Danil-Grigorev commented May 14, 2021 •
edited
Loading

Adds support on Azure and AzureStackHub for IsCloudProviderExternal helper. Depends on api change to be merged first: openshift/api#860

@openshift-ci openshift-ci Bot requested review from deads2k and sttts May 14, 2021 15:56
@Danil-Grigorev Danil-Grigorev changed the title Include Azure support to external CCM selection [OCPCLOUD-1187] Include Azure support to external CCM selection Jun 14, 2021
@Danil-Grigorev Danil-Grigorev mentioned this pull request Jun 15, 2021
Merged
JoelSpeed
JoelSpeed reviewed Jun 15, 2021
View reviewed changes
Comment thread pkg/cloudprovider/external.go Outdated
Comment thread pkg/cloudprovider/external.go Outdated
Comment thread pkg/cloudprovider/external.go Outdated
@Danil-Grigorev Danil-Grigorev requested a review from JoelSpeed June 15, 2021 11:55
JoelSpeed
JoelSpeed reviewed Jun 15, 2021
View reviewed changes
Copy link
Copy Markdown
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.

Please make sure there's a test for when the platform status is nil, otherwise LGTM

@Danil-Grigorev Danil-Grigorev requested a review from JoelSpeed June 16, 2021 11:24
JoelSpeed
JoelSpeed reviewed Jun 16, 2021
View reviewed changes
Comment thread pkg/cloudprovider/external_test.go
@Danil-Grigorev Danil-Grigorev force-pushed the external-azure branch 2 times, most recently from 2a1707a to 05d070b Compare June 16, 2021 18:17
@Danil-Grigorev Danil-Grigorev requested a review from JoelSpeed June 16, 2021 18:18
@Danil-Grigorev
Copy link
Copy Markdown
Author

Danil-Grigorev commented Jun 17, 2021

/retest

@JoelSpeed
Copy link
Copy Markdown
Contributor

JoelSpeed commented Jun 17, 2021

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jun 17, 2021
@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 17, 2021
@JoelSpeed
Copy link
Copy Markdown
Contributor

JoelSpeed commented Jun 18, 2021

/lgtm

Danil and I had some discussion yesterday about the bindata and podnetworkconnectivity check yaml, seems that the verify job just ensures that the files are synced with the vendor folder, which has been updated to include the latest changes in openshift/api.

Looking for an owner to confirm that copying them was the right thing to do

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jun 18, 2021
soltysh
soltysh approved these changes Jun 25, 2021
View reviewed changes
Copy link
Copy Markdown

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

/lgtm

This was referenced Jun 29, 2021
Merged
Merged
@sttts
Copy link
Copy Markdown
Contributor

sttts commented Jul 2, 2021

/approve

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 2, 2021
Revendor with openshift/api change for AzureStackHub
88b73d2 
- Update bindata for network crd
@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 2, 2021
@Danil-Grigorev
Copy link
Copy Markdown
Author

Danil-Grigorev commented Jul 2, 2021

/retest

@JoelSpeed
Copy link
Copy Markdown
Contributor

JoelSpeed commented Jul 2, 2021

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 2, 2021
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jul 2, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Danil-Grigorev, JoelSpeed, soltysh, sttts

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 39570b4 into openshift:master Jul 2, 2021
@Danil-Grigorev Danil-Grigorev mentioned this pull request Jul 2, 2021
Merged
@lobziik lobziik mentioned this pull request Jul 9, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@deads2k deads2k Awaiting requested review from deads2k

@sttts sttts Awaiting requested review from sttts

@JoelSpeed JoelSpeed Awaiting requested review from JoelSpeed

1 more reviewer

@soltysh soltysh soltysh approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@soltysh soltysh

@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

@Danil-Grigorev @JoelSpeed @sttts @soltysh @openshift-merge-robot