HOSTEDCP-1401: Enable BYO NSG #3455
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@Patryk-Stefanski: This pull request references HOSTEDCP-1294 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target the "4.16.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
openshift-ci
bot
added
do-not-merge/needs-area
area/cli
Patryk-Stefanski
changed the title
|
@Patryk-Stefanski: This pull request references HOSTEDCP-1401 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/test all |
| securityGroupID = *nsg.ID | ||
| result.SecurityGroupName = o.NetworkSecurityGroup | ||
| l.Info("Successfully found existing network security group", "name", o.NetworkSecurityGroup) | ||
| } else { |
There was a problem hiding this comment.
We don't want to create it in this case since this is BYO NSG
There was a problem hiding this comment.
This only creates the security group if opts.NetworkSecurityGroup is an empty string
There was a problem hiding this comment.
We still don't want to create it though. We already create a NSG in the infra code -
hypershift/cmd/infra/azure/create.go
Line 152 in 964d9fe
There was a problem hiding this comment.
We should do something similar to RG. Here's the reference for that -
hypershift/cmd/infra/azure/create.go
Line 217 in 964d9fe
There was a problem hiding this comment.
Nevermind, I was thinking we were in a different file SMH
|
/test all |
Patryk-Stefanski
force-pushed
the
HOSTEDCP-1294
branch
from
cbe6329
to
446a23a
Compare
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
@Patryk-Stefanski: This pull request references HOSTEDCP-1401 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
Patryk-Stefanski
force-pushed
the
HOSTEDCP-1294
branch
from
446a23a
to
c6816d2
Compare
|
/test verify |
|
/retest-required |
Patryk-Stefanski
force-pushed
the
HOSTEDCP-1294
branch
from
c6816d2
to
69e4d16
Compare
| } | ||
| result.SecurityGroupName = securityGroupName | ||
| l.Info("Successfully created network security group", "name", securityGroupName) |
There was a problem hiding this comment.
You could just print out result.SecurityGroupName here and remove the print outs from the if/else.
There was a problem hiding this comment.
That would leave you with a misleading message. If you specifed the security group you want to use it would then tell you that it has created a security group.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox, Patryk-Stefanski The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/lgtm |
|
/retest |
|
@Patryk-Stefanski: all tests passed! Full PR test history. Your PR dashboard. 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. |
|
[ART PR BUILD NOTIFIER] This PR has been included in build ose-hypershift-container-v4.16.0-202401272017.p0.gf9c0490.assembly.stream for distgit hypershift. |
What this PR does / why we need it: Add BYO network security group capability to Azure
Which issue(s) this PR fixes (optional, use
fixes #<issue_number>(, fixes #<issue_number>, ...)format, where issue_number might be a GitHub issue, or a Jira story:Fixes # HOSTEDCP-1401
Checklist