Support for lifecycles #2763

justinsb · 2017-06-16T05:26:55Z

This change is

chrislovecnm

Questions for you

chrislovecnm · 2017-06-16T21:37:16Z

cmd/kops/update_cluster.go

@@ -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")


Should we have this as a list? So I can run "iam,network" for instance?

Can you explain the use case? Just convenience?

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.

When testing this I am noticing that we are not printing the allowed values for Phase. Can we add to update_cluster.go?

We can add those later, let's start trying this out for real and then see whether we need it

chrislovecnm · 2017-06-16T21:39:27Z

upup/models/cloudup/tokens/tokens.yaml

@@ -1,22 +0,0 @@
-secret/kube: {}


Yay more of this stuff removed!

chrislovecnm · 2017-06-16T21:40:54Z

upup/pkg/fi/cloudup/apply_cluster.go

+	networkLifecycle := lifecyclePointer(fi.LifecycleSync)
+	clusterLifecycle := lifecyclePointer(fi.LifecycleSync)
+
+	switch c.Phase {


I would definitely like to have the capability to run both PhaseNetwork and PhaseCluster at the same time.

Can you explain why? In particular, why not just run two commands?

A goal here is to eventually make networks a top level object, like Cluster.

chrislovecnm · 2017-06-16T21:43:00Z

upup/pkg/fi/cloudup/bootstrapchannelbuilder.go

@@ -28,7 +28,8 @@ import (
 )

 type BootstrapChannelBuilder struct {
-	cluster *kops.Cluster
+	cluster   *kops.Cluster
+	Lifecycle *fi.Lifecycle


Crazy question, should bootstrap be another lifecycle? This would allow for the easier upgrade of stuff like CNI providers.

I think bootstrap is pretty tightly bound to the rest of the cluster. And if only the CNI has changed, then I think that our logic should ensure that only that is actually changed...

chrislovecnm · 2017-06-16T21:44:22Z

upup/pkg/fi/context.go

@@ -99,6 +100,57 @@ func (c *Context) NewTempDir(prefix string) (string, error) {
 var typeContextPtr = reflect.TypeOf((*Context)(nil))

 func (c *Context) Render(a, e, changes Task) error {


function documentation?

chrislovecnm · 2017-06-16T21:45:37Z

cmd/kops/update_cluster.go

@@ -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


Should we have the phase options lower, say in create.go? I will probably need kops create -f mycluster.yaml --phase iam.

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?

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.

Let's get this in and look at the broader role of create vs update as part of the kops-server UX pass

chrislovecnm · 2017-07-05T22:30:30Z

Getting message from update: "Cluster is starting. It should be ready in a few minutes." After running --phase iam. We probably should tweak the output. I can file an issue on it, and have a follow-up PR.

chrislovecnm

I am also seeing issues with the shared VPC, and subnets being deleted. I know the shared VPCs being delete is an open issue, but now we are deleting the VPC 😞

I am not certain this is an issue with this PR, I am going to test with master as well.

chrislovecnm

Would like LifecycleExistsAndWarnIfChanges updated, and other nit picks.

Looks awesome, what is the plan for getting this in?

chrislovecnm · 2017-07-12T19:20:00Z

upup/pkg/fi/cloudup/apply_cluster.go

+			networkLifecycle = lifecyclePointer(fi.LifecycleExistsAndWarnIfChanges)
+		} else {
+			iamLifecycle = lifecyclePointer(fi.LifecycleExistsAndValidates)
+			networkLifecycle = lifecyclePointer(fi.LifecycleExistsAndValidates)


Can we loosen these and change them to LifecycleExistsAndWarnIfChanges. Allowing this will fix multiple scenarios including not using an internet gateway.

Not using an internet gateway is orthogonal, but I think it would be great to have more custom topology support.

justinsb · 2017-07-16T02:04:36Z

Added comment on the Render function. Rebased.

Subnet deletion is probably pre-existing. Will send a PR now.

chrislovecnm · 2017-07-16T18:26:09Z

Related #1576

chrislovecnm

Thanks

chrislovecnm · 2017-07-21T16:07:43Z

The one challenge that we have is that network check may need to be LifecycleExistsAndWarnIfChanges not a hard check.

chrislovecnm · 2017-07-21T16:07:51Z

/lgtm

k8s-github-robot · 2017-07-21T16:27:27Z

/test all [submit-queue is verifying that this PR is safe to merge]

k8s-github-robot · 2017-07-21T16:58:59Z

Automatic merge from submit-queue

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 16, 2017

justinsb force-pushed the lifecycle branch from e1060e6 to e611762 Compare June 16, 2017 05:33

chrislovecnm reviewed Jun 16, 2017

View reviewed changes

justinsb force-pushed the lifecycle branch from e611762 to 2eefd60 Compare July 1, 2017 20:30

justinsb changed the title ~~WIP: Support for lifecycles~~ Support for lifecycles Jul 2, 2017

chrislovecnm suggested changes Jul 5, 2017

View reviewed changes

chrislovecnm suggested changes Jul 12, 2017

View reviewed changes

justinsb mentioned this pull request Jul 16, 2017

Allow the re-use of a IAM role that is not created by kops. #2440

Closed

5 tasks

justinsb added 4 commits July 15, 2017 22:01

Add Lifecycle

3e1ce4f

Wiring up lifecycle

3dfe48e

autogen

ebc97af

Document Render function

7a03c66

justinsb force-pushed the lifecycle branch from 62cb0bc to 7a03c66 Compare July 16, 2017 02:04

chrislovecnm approved these changes Jul 21, 2017

View reviewed changes

k8s-ci-robot assigned chrislovecnm Jul 21, 2017

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 21, 2017

k8s-github-robot merged commit 62d702a into kubernetes:master Jul 21, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for lifecycles #2763

Support for lifecycles #2763

justinsb commented Jun 16, 2017 •

edited by k8s-reviewable

Loading

chrislovecnm left a comment

chrislovecnm Jun 16, 2017

justinsb Jul 1, 2017

chrislovecnm Jul 3, 2017

chrislovecnm Jul 5, 2017

justinsb Jul 16, 2017

chrislovecnm Jun 16, 2017

chrislovecnm Jun 16, 2017

justinsb Jul 1, 2017

chrislovecnm Jun 16, 2017

justinsb Jul 1, 2017

chrislovecnm Jun 16, 2017

chrislovecnm Jun 16, 2017

justinsb Jul 1, 2017

chrislovecnm Jul 3, 2017

justinsb Jul 16, 2017

chrislovecnm commented Jul 5, 2017

chrislovecnm left a comment •

edited

Loading

chrislovecnm left a comment

chrislovecnm Jul 12, 2017

justinsb Jul 16, 2017

justinsb commented Jul 16, 2017

chrislovecnm commented Jul 16, 2017

chrislovecnm left a comment

chrislovecnm commented Jul 21, 2017

chrislovecnm commented Jul 21, 2017

k8s-github-robot commented Jul 21, 2017

k8s-github-robot commented Jul 21, 2017

		@@ -99,6 +100,57 @@ func (c *Context) NewTempDir(prefix string) (string, error) {
		var typeContextPtr = reflect.TypeOf((*Context)(nil))

		func (c *Context) Render(a, e, changes Task) error {

Support for lifecycles #2763

Support for lifecycles #2763

Conversation

justinsb commented Jun 16, 2017 • edited by k8s-reviewable Loading

chrislovecnm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrislovecnm commented Jul 5, 2017

chrislovecnm left a comment • edited Loading

Choose a reason for hiding this comment

chrislovecnm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinsb commented Jul 16, 2017

chrislovecnm commented Jul 16, 2017

chrislovecnm left a comment

Choose a reason for hiding this comment

chrislovecnm commented Jul 21, 2017

chrislovecnm commented Jul 21, 2017

k8s-github-robot commented Jul 21, 2017

k8s-github-robot commented Jul 21, 2017

justinsb commented Jun 16, 2017 •

edited by k8s-reviewable

Loading

chrislovecnm left a comment •

edited

Loading