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.

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

Add globalnet-cluster-size option to join #288

Merged
merged 2 commits into from
Apr 8, 2020

Conversation

vthapar
Copy link
Contributor

@vthapar vthapar commented Mar 27, 2020

Currently --globalnet-cluster-size can be specified only during broker
installation. This adds option to give a different size during join.

Closes #268

@submariner-bot
Copy link
Contributor

🤖 Created branch: z_pr288/vthapar/globalnet-issue-268

@submariner-bot
Copy link
Contributor

🤖 Updated branch: z_pr288/vthapar/globalnet-issue-268

Copy link
Member

@sridhargaddam sridhargaddam left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -90,6 +90,8 @@ func addJoinFlags(cmd *cobra.Command) {
cmd.Flags().StringVar(&cableDriver, "cable-driver", "", "Cable driver implementation")
cmd.Flags().BoolVar(&disableOpenShiftCVO, "disable-cvo", false,
"disable OpenShift's cluster version operator if necessary, without prompting")
cmd.Flags().UintVar(&globalnetClusterSize, "globalnet-cluster-size", 0,
Copy link
Member

Choose a reason for hiding this comment

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

I think in future we may want to support specifying an all-together different CIDR as part of join operation via globalnet-cidr flag. This is outside the scope of the current PR.

Copy link
Member

Choose a reason for hiding this comment

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

Just for record - #298

Copy link
Member

@dfarrell07 dfarrell07 left a comment

Choose a reason for hiding this comment

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

Needs a manual rebase, and might need another after additional Shipyard migrations go in soon.

@submariner-bot
Copy link
Contributor

🤖 Updated branch: z_pr288/vthapar/globalnet-issue-268

@submariner-bot
Copy link
Contributor

🤖 Updated branch: z_pr288/vthapar/globalnet-issue-268

@submariner-bot
Copy link
Contributor

🤖 Updated branch: z_pr288/vthapar/globalnet-issue-268

@submariner-bot
Copy link
Contributor

🤖 Updated branch: z_pr288/vthapar/globalnet-issue-268

@submariner-bot
Copy link
Contributor

🤖 Updated branch: z_pr288/vthapar/globalnet-issue-268

Currently --globalnet-cluster-size can be specified only during broker
installation. This adds option to give a different size during join.

Closes submariner-io#268
@submariner-bot
Copy link
Contributor

🤖 Updated branch: z_pr288/vthapar/globalnet-issue-268

Copy link
Member

@sridhargaddam sridhargaddam left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for addressing this @vthapar

@@ -90,6 +90,8 @@ func addJoinFlags(cmd *cobra.Command) {
cmd.Flags().StringVar(&cableDriver, "cable-driver", "", "Cable driver implementation")
cmd.Flags().BoolVar(&disableOpenShiftCVO, "disable-cvo", false,
"disable OpenShift's cluster version operator if necessary, without prompting")
cmd.Flags().UintVar(&globalnetClusterSize, "globalnet-cluster-size", 0,
Copy link
Member

Choose a reason for hiding this comment

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

Just for record - #298

Copy link
Contributor

@mangelajo mangelajo left a comment

Choose a reason for hiding this comment

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

2 tiny nitpickings to clarify the parameters

pkg/subctl/cmd/deploybroker.go Outdated Show resolved Hide resolved
pkg/subctl/cmd/join.go Outdated Show resolved Hide resolved
Co-Authored-By: Miguel Angel Ajo Pelayo <miguelangel@ajo.es>
@submariner-bot
Copy link
Contributor

🤖 Updated branch: z_pr288/vthapar/globalnet-issue-268

@submariner-bot
Copy link
Contributor

🤖 I had an issue pushing the updated branch: command error on refs/heads/z_pr288/vthapar/globalnet-issue-268: cannot lock ref 'refs/heads/z_pr288/vthapar/globalnet-issue-268': is at 9d3bbde but expected 962df34

@mangelajo mangelajo merged commit 54f6b6b into submariner-io:master Apr 8, 2020
@submariner-bot
Copy link
Contributor

🤖 Closed branches: [z_pr288/vthapar/globalnet-issue-268]

@vthapar vthapar deleted the globalnet-issue-268 branch September 23, 2020 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provision to specify globalnet-cluster-size during subctl join
5 participants