Skip to content
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

Get Public Cloud instance public IP when needed #21141

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Get Public Cloud instance public IP when needed #21141

wants to merge 1 commit into from
+82 −55

Conversation

pdostal
Copy link
Member

@pdostal pdostal commented Feb 7, 2025

@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 7, 2025

Great PR! Please pay attention to the following items before merging:

Files matching lib/publiccloud/**.pm:

  • Provide VRs for both QE-C as well as QE-SAP (check Confluence for more info)

This is an automatically generated QA checklist based on modified files.

@pdostal pdostal added the blocked Blocked from another task label Feb 7, 2025
asmorodskyi
asmorodskyi reviewed Feb 7, 2025
View reviewed changes
lib/publiccloud/provider.pm
@@ -618,6 +619,7 @@ sub terraform_apply {
foreach my $i (0 .. $#{$vms}) {
my $instance = publiccloud::instance->new(
public_ip => @{$ips}[$i],
resource_group => $resource_group,
Copy link
Member

Choose a reason for hiding this comment

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

but resource group is relevant only for Azure and this code is used by all 3 providers 🤔

asmorodskyi
asmorodskyi approved these changes Feb 7, 2025
View reviewed changes
Copy link
Member

@asmorodskyi asmorodskyi left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asmorodskyi asmorodskyi asmorodskyi approved these changes

@grisu48 grisu48 Awaiting requested review from grisu48 grisu48 is a code owner

@ricardobranco777 ricardobranco777 Awaiting requested review from ricardobranco777 ricardobranco777 is a code owner

@mpagot mpagot Awaiting requested review from mpagot mpagot is a code owner

@m-dati m-dati Awaiting requested review from m-dati m-dati is a code owner

@alvarocarvajald alvarocarvajald Awaiting requested review from alvarocarvajald alvarocarvajald is a code owner

@BillAnastasiadis BillAnastasiadis Awaiting requested review from BillAnastasiadis BillAnastasiadis is a code owner

@rbmarliere rbmarliere Awaiting requested review from rbmarliere rbmarliere is a code owner

Assignees
No one assigned
Labels
blocked Blocked from another task
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pdostal @asmorodskyi