k8saas: better kubeconfig support #372
Conversation
aybabtme
force-pushed
the
k8saas-support
branch
2 times, most recently
from
b0a5195
to
1200693
Compare
| if err := writeOrAddToKubeconfig(kubeconfig); err != nil { | ||
| warn(fmt.Sprintf("couldn't write cluster credentials: %v", err)) | ||
| } | ||
| } |
There was a problem hiding this comment.
Why don't you return an error and print it here instead ?
There was a problem hiding this comment.
i could, but it'd be redundant. also because this isn't a reason to fail the whole operation and not print the cluster that was just created, it would confuse users into thinking their cluster wasn't created
| failCount = 0 // API responded, reset it's error counter | ||
| } | ||
| if cluster.Status == nil { | ||
| time.Sleep(1 * time.Second) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
|
||
| // waitForClusterRunning waits for a cluster to be running. | ||
| func waitForClusterRunning(kube do.KubernetesService, clusterID string) error { | ||
| failCount := 0 |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
aybabtme
force-pushed
the
k8saas-support
branch
5 times, most recently
from
50b29dd
to
5c81229
Compare
aybabtme
force-pushed
the
k8saas-support
branch
from
887f7b7
to
c28d2c1
Compare
| AddStringFlag(cmdKubeClusterCreate, doctl.ArgSizeSlug, "", defaultNodeSize, "size of the nodes in the default node pool (incompatible with --"+doctl.ArgClusterNodePool+")") | ||
| AddStringFlag(cmdKubeClusterCreate, doctl.ArgNodePoolCount, "", strconv.Itoa(defaultNodeCount), "size of the nodes in the default node pool (incompatible with --"+doctl.ArgClusterNodePool+")") | ||
| AddStringSliceFlag(cmdKubeClusterCreate, doctl.ArgClusterNodePool, "", nil, `cluster node pools in the form "name=your-name;size=droplet_size;count=5;tag=tag1;tag=tag2"`, requiredOpt()) | ||
| AddBoolFlag(cmdKubeClusterCreate, doctl.ArgClusterUpdateKubeconfig, "", true, "whether to add the created cluster to your kubeconfig") |
There was a problem hiding this comment.
This default also changes the current context to the new cluster cluster, should this maybe be a separate value that defaults to false? I've had a few occasions where this happened and I forgot about it, and got very confused why my kubectl seemed to not be doing what I wanted it to the cluster that was previously my active context.
There was a problem hiding this comment.
I'm reproducing the minikube and kubeadm behavior here, but this could be a flag yes.
There was a problem hiding this comment.
Is there a way you could make the node-pool flag more usable or show how to print the various "slug" sizes in the help text?
There was a problem hiding this comment.
Yeah I'll try to make this better in a next iteration.
aybabtme
force-pushed
the
k8saas-support
branch
from
a1ffcc1
to
4e1a573
Compare
|
Looks (and feels) ready to me. |
I know minikube does this, but is there a flag to preserve context? When creating clusters with eksctl for example I'm not forced to get a context-switch. |
|
@alexellis I'll add one, and change the default behavior to not change the context. It seems that enough people find this jarring to suggest it's probably not a good default behavior. |
|
actually I'll just remove that changing the context thing, it's already easily done with |
runningon acreatecall.