-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Support for lifecycles #2763
Support for lifecycles #2763
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,6 +65,8 @@ type UpdateClusterOptions struct { | |
SSHPublicKey string | ||
MaxTaskDuration time.Duration | ||
CreateKubecfg bool | ||
|
||
Phase string | ||
} | ||
|
||
func (o *UpdateClusterOptions) InitDefaults() { | ||
|
@@ -107,6 +109,7 @@ func NewCmdUpdateCluster(f *util.Factory, out io.Writer) *cobra.Command { | |
cmd.Flags().StringVar(&options.SSHPublicKey, "ssh-public-key", options.SSHPublicKey, "SSH public key to use (deprecated: use kops create secret instead)") | ||
cmd.Flags().StringVar(&options.OutDir, "out", options.OutDir, "Path to write any local output") | ||
cmd.Flags().BoolVar(&options.CreateKubecfg, "create-kube-config", options.CreateKubecfg, "Will control automatically creating the kube config file on your local filesystem") | ||
cmd.Flags().StringVar(&options.Phase, "phase", options.Phase, "Subset of tasks to run") | ||
return cmd | ||
} | ||
|
||
|
@@ -172,6 +175,20 @@ func RunUpdateCluster(f *util.Factory, clusterName string, out io.Writer, c *Upd | |
glog.Infof("Using SSH public key: %v\n", c.SSHPublicKey) | ||
} | ||
|
||
var phase cloudup.Phase | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we have the phase options lower, say in create.go? I will probably need There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Phase is really only for updating. Today there are some circumstances in which create will do an update (for historical reasons), and you have a PR to tweak that, and the kops-server work may also change that (because in k8s today, creating an object immediately does an update). I'd prefer to leave our options open unless we know we need it today. Do we need it today? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the majority of user that would be interested in this would be interesting with this with yaml support. We can merge and move lower if you like. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's get this in and look at the broader role of create vs update as part of the kops-server UX pass |
||
if c.Phase != "" { | ||
switch strings.ToLower(c.Phase) { | ||
case "iam": | ||
phase = cloudup.PhaseIAM | ||
case "network": | ||
phase = cloudup.PhaseNetwork | ||
case "cluster": | ||
phase = cloudup.PhaseCluster | ||
default: | ||
return fmt.Errorf("unknown phase %q", c.Phase) | ||
} | ||
} | ||
|
||
var instanceGroups []*kops.InstanceGroup | ||
{ | ||
list, err := clientset.InstanceGroupsFor(cluster).List(metav1.ListOptions{}) | ||
|
@@ -192,6 +209,7 @@ func RunUpdateCluster(f *util.Factory, clusterName string, out io.Writer, c *Upd | |
DryRun: isDryrun, | ||
MaxTaskDuration: c.MaxTaskDuration, | ||
InstanceGroups: instanceGroups, | ||
Phase: phase, | ||
} | ||
|
||
err = applyCmd.Run() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we have this as a list? So I can run "iam,network" for instance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain the use case? Just convenience?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For instance many companies have users that have permissions for iam and network, and another group has perms for ec2 instances. We have many users in this situation. It would be god to provide a user to run a single command to create iam and network.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When testing this I am noticing that we are not printing the allowed values for
Phase
. Can we add to update_cluster.go?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can add those later, let's start trying this out for real and then see whether we need it