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

Select a single subnet for a single AZ machine pool - BYOVPC clusters #765

Merged
merged 1 commit into from
Jun 30, 2022
Merged

Select a single subnet for a single AZ machine pool - BYOVPC clusters #765

merged 1 commit into from
Jun 30, 2022

Conversation

oriAdler
Copy link
Contributor

@oriAdler oriAdler commented Jun 29, 2022 •
edited
Loading

  1. Add the subnet flag to the create machine pool command.
  2. Add functions to fetch VPC's private subnets.

Possible flows:

  1. BYOVPC single AZ cluster - the user is prompted with the option to select subnet.
  2. BYOVPC multi-AZ cluster - if the user chooses a single AZ, they have the option
    to provide a subnet or an availability zone.

Related: SDA-6289

select_subnet_for_single_az_machine_pool.webm

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 29, 2022
@openshift-ci openshift-ci bot requested review from ciaranRoche and vkareh June 29, 2022 11:07
@oriAdler oriAdler force-pushed the select-subnet-for-single-az-machine-pool branch from 48b1473 to 75b8bcd Compare June 29, 2022 13:44
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 29, 2022
@oriAdler oriAdler force-pushed the select-subnet-for-single-az-machine-pool branch 2 times, most recently from 65470de to 07ae6e7 Compare June 29, 2022 14:16
@oriAdler
Copy link
Contributor Author

/retest

@tbrisker
Copy link
Contributor

@oriAdler can you please split out the sdk upgrade from the actual code change? And if the code change has two parts, splitting that into smaller commits/separate PRs makes review much easier.

@oriAdler
Copy link
Contributor Author

@oriAdler can you please split out the sdk upgrade from the actual code change? And if the code change has two parts, splitting that into smaller commits/separate PRs makes review much easier.
@tbrisker sure, I'll create a separate MR for the SDK upgrade.

@oriAdler oriAdler force-pushed the select-subnet-for-single-az-machine-pool branch from 7194304 to 9dd152c Compare June 30, 2022 07:42
@oriAdler oriAdler mentioned this pull request Jun 30, 2022
Merged
@oriAdler oriAdler marked this pull request as draft June 30, 2022 09:40
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 30, 2022
@oriAdler oriAdler force-pushed the select-subnet-for-single-az-machine-pool branch from 9dd152c to eef97dc Compare June 30, 2022 11:09
@oriAdler
Select a single subnet for a single AZ machine pool - BYOVPC clusters
8c06c00 
1. Update `omc-sdk-go` to v0.1.275.
2. Add `subnet` flag to `create machine pool` command.
3. Add functions to fetch VPC's private subnets.

Possible flows:
1. BYOVPC single AZ cluster - the user is prompted with the option to select subnet.
2. BYOVPC multi-AZ cluster - if the user chooses a single AZ, they have the option
to provide subnet or an availability zone.

Related: SDA-6289
@oriAdler oriAdler force-pushed the select-subnet-for-single-az-machine-pool branch from eef97dc to 8c06c00 Compare June 30, 2022 11:12
@oriAdler oriAdler marked this pull request as ready for review June 30, 2022 11:13
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 30, 2022
@openshift-ci openshift-ci bot requested a review from igoihman June 30, 2022 11:24
@oriAdler
Copy link
Contributor Author

@zgalor @pvasant @tbrisker PTAL

@pvasant
Copy link
Contributor

pvasant commented Jun 30, 2022

LGTM @zgalor please stamp it once your are done reviewing

@zgalor
Copy link
Contributor

zgalor commented Jun 30, 2022

LGTM

@zgalor
Copy link
Contributor

zgalor commented Jun 30, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 30, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 30, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: oriAdler, zgalor

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 30, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 30, 2022

@oriAdler: 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.

@openshift-ci openshift-ci bot merged commit 8b162b0 into openshift:master Jun 30, 2022
@oriAdler oriAdler deleted the select-subnet-for-single-az-machine-pool branch July 3, 2022 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ciaranRoche ciaranRoche Awaiting requested review from ciaranRoche

@vkareh vkareh Awaiting requested review from vkareh

@igoihman igoihman Awaiting requested review from igoihman

Assignees

@zgalor zgalor

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.
4 participants
@oriAdler @tbrisker @pvasant @zgalor