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

[clusterctl] add phases create-bootstrap-cluster subcommand #553

Merged
merged 1 commit into from
Nov 2, 2018
Merged

[clusterctl] add phases create-bootstrap-cluster subcommand #553

merged 1 commit into from
Nov 2, 2018

Conversation

detiber
Copy link
Member

@detiber detiber commented Oct 18, 2018 •
edited
Loading

What this PR does / why we need it:

Starts to add phases support to clusterctl to allow for a user to run a subset of steps.

Related to: #554

Release note:

- `clusterctl` now has an alpha phases create-bootstrap-cluster subcommand

@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 18, 2018
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 18, 2018
@detiber
Copy link
Member Author

detiber commented Oct 18, 2018

@randomvariable @ashish-amarnath @vincepri @chuckha ptal, my goal here is to eventually have all of the pieces of clusterctl create broken out into individual phases so that we can help reduce some of the confusion that occurs with the current workflow.

@detiber detiber changed the title [WIP] [clusterctl] start adding phase support [clusterctl] start adding phase support Oct 18, 2018
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 18, 2018
@detiber detiber changed the title [clusterctl] start adding phase support [clusterctl] add phases create-bootstrap-cluster subcommand Oct 18, 2018
@randomvariable
Copy link
Member

This is exactly what we need.

@vincepri
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 18, 2018
@ashish-amarnath
Copy link
Contributor

/lgtm

@k8s-ci-robot
Copy link
Contributor

@ashish-amarnath: changing LGTM is restricted to assignees, and only kubernetes-sigs/cluster-api repo collaborators may be assigned issues.

In response to this:

/lgtm

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.

@detiber detiber mentioned this pull request Oct 19, 2018
Closed
11 tasks
@ingvagabund
Copy link
Contributor

The phases layout is fine, most of the changes are about moving code so the clusterctl phases create-bootstrap-cluster can be run separately.
/lgtm

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 26, 2018
@davidewatson
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 31, 2018
roberthbailey
roberthbailey reviewed Oct 31, 2018
View reviewed changes
Copy link
Contributor

@roberthbailey roberthbailey left a comment

Choose a reason for hiding this comment

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

/lgtm

cmd/clusterctl/cmd/alpha_phase_create_bootstrap_cluster.go Outdated Show resolved Hide resolved
cmd/clusterctl/clusterdeployer/clusterdeployer.go Outdated Show resolved Hide resolved
cmd/clusterctl/phases/createbootstrapcluster.go Show resolved Hide resolved
@roberthbailey
Copy link
Contributor

I left a few nits about import grouping. Holding for ~24 hours as discussed on the call today.

/approve
/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 31, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: detiber, roberthbailey

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 31, 2018
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 31, 2018
@detiber
Copy link
Member Author

detiber commented Oct 31, 2018

/test pull-cluster-api-test

1 similar comment
@detiber
Copy link
Member Author

detiber commented Oct 31, 2018

/test pull-cluster-api-test

@ingvagabund
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 1, 2018
@roberthbailey
Copy link
Contributor

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 2, 2018
@detiber
[clusterctl] start adding phase support
11841f8 
- add a separate `clusterctl alpha phases create-bootstrap-cluster` command
- move bootstrap cluster creation steps out of clusterdeployer
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 2, 2018
@detiber
Copy link
Member Author

detiber commented Nov 2, 2018

Rebased to resolve build/test failure that cropped up due to the arguments that were added to clusterctl delete cluster

@detiber
Copy link
Member Author

detiber commented Nov 2, 2018

/test pull-cluster-api-test

@roberthbailey
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 2, 2018
@k8s-ci-robot k8s-ci-robot merged commit 571cb48 into kubernetes-sigs:master Nov 2, 2018
@detiber detiber deleted the phases branch April 2, 2019 17:44
jayunit100 pushed a commit to jayunit100/cluster-api that referenced this pull request Jan 31, 2020
@k8s-ci-robot
Merge pull request kubernetes-sigs#553 from ykakarap/delete_vms
a16f7d6 
remove MachineRef when vm is deleted
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roberthbailey roberthbailey roberthbailey left review comments

@vincepri vincepri vincepri approved these changes

@krousey krousey Awaiting requested review from krousey

Assignees

@vincepri vincepri

@roberthbailey roberthbailey

@ingvagabund ingvagabund

@davidewatson davidewatson

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@detiber @randomvariable @vincepri @ashish-amarnath @k8s-ci-robot @ingvagabund @davidewatson @roberthbailey