Extend kubeconfig writer to support global path with merging #80

errordeveloper · 2018-06-22T16:59:10Z

Incorporates @richardcase's changes from #78.

errordeveloper · 2018-06-22T17:15:37Z

@richardcase so I couldn't stop wondering if k8s.io/client-go/tools/clientcmd should be able to do the merging for us, but all references to "merge" in the doc seem to refer to merging of configs on load...

So I looked at github.com/kubicorn/kubicorn/pkg/kubeconfig and that seems to be very specific to kubicorn and mostly implements whatever is needed to fetch the config via SSH.

I also looked at github.com/kubernetes/kops/pkg/kubeconfig and discovered that it is also very specific to kops, but it uses clientcmd.ModifyConfig which looks intriguing.

errordeveloper · 2018-06-23T14:15:13Z

I've reviewed the kubeconfig package in kops, I think we should be able to use it actually, but it might need some refactoring (it uses glog and fmt, but that should be easy to fix upstream). It seems like the package does what we want, I'll need to take another read to confirm that 100%. I'm also very happy to merge this to start with.

richardcase · 2018-06-25T06:43:46Z

@errordeveloper - you are right KubeconfigBuilder from kops would work. I like the fact that it uses ModifyConfig to do the read of existing and saving of the modified config. As you say very intriguing.

We could refactor to use it directly or change our code to do something similar and call ModifyConfig. I'm happy to do either of these 2 if you want.

richardcase · 2018-06-25T06:58:16Z

As you pointed out the kops implementation use glog and fmt and it doesn't support ExecConfig that we require for Heptio Authenticator (unless i've missed this).

I wonder if we should refactor our implementation to use a similar approach to kops and take advantage of clientcmd.NewDefaultPathOptions, clientcmd.ModifyConfig and also clientcmd.GetStartingConfig.

I also like that they do a delete of the cluster, context and authinfos. Our delete logic is a bit crude at the moment (i.e. only delete an autogenerated config).

What do you think?

errordeveloper · 2018-06-25T12:50:59Z

So I've missed ExecConfig part. I think refactoring to make use of clientcmd.NewDefaultPathOptions, clientcmd.ModifyConfig and clientcmd.GetStartingConfig, as you said, is a good idea. Perhaps we can get that in, and open an issue to coverage with kops later.

richardcase · 2018-06-25T21:13:25Z

I will refactor based on the above against the issue-29-merge-kubeconfig branch.

richardcase · 2018-06-30T05:06:29Z

As discussed I've updated our implementation to use clientcmd.NewDefaultPathOptions, clientcmd.ModifyConfig and clientcmd.GetStartingConfig.

I will raise an issue with kops to support ExecConfig and link it to an issue here to track and change our implementation.

errordeveloper · 2018-07-03T15:12:39Z

@richardcase I'll test this tomorrow and merge

pkg/utils/kubeconfig/kubeconfig_test.go

@@ -23,7 +23,7 @@ func TestCreateNewKubeConfig(t *testing.T) {
 			"test-context": {AuthInfo: "test-user", Cluster: "test-cluster", Namespace: "test-ns"}},
 	}

-	err := kubeconfig.WriteToFile(configFile.Name(), &testConfig, false)
+	err := kubeconfig.WriteKubeCfg(configFile.Name(), &testConfig, false)


errordeveloper · 2018-07-04T16:23:41Z

I've spotted a couple of issues, turns out NewClientSetWithEmbeddedToken expects current-context to be set, also utils.CheckAllCommands will break... I'm testing a fix a the moment, will push once I confirm it works.

errordeveloper · 2018-07-04T17:45:37Z

@richardcase please take a look

@jstrachan FYI

richardcase

This looks good to me.

cmd/eksctl/create.go

@@ -79,7 +79,7 @@ func createClusterCmd() *cobra.Command {

 	fs.BoolVar(&writeKubeconfig, "write-kubeconfig", true, "toggle writing of kubeconfig")
 	fs.BoolVar(&autoKubeconfigPath, "auto-kubeconfig", false, fmt.Sprintf("save kubconfig file by cluster name, e.g. %q", kubeconfig.AutoPath(exampleClusterName)))
-	fs.StringVar(&kubeconfigPath, "kubeconfig", "", "path to write kubeconfig (incompatible with --auto-kubeconfig)")
+	fs.StringVar(&kubeconfigPath, "kubeconfig", kubeconfig.DefaultPath, "path to write kubeconfig (incompatible with --auto-kubeconfig)")


pkg/utils/kubeconfig/kubeconfig.go

 // Write will write Kubernetes client configuration to a file.
 // If path isn't specified then the path will be determined by client-go.
 // If file pointed to by path doesn't exist it will be created.
 // If the file already exists then the configuration will be merged with the existing file.
-func Write(path string, newConfig *api.Config, setContext bool) error {
+func Write(path string, newConfig *api.Config, setContext bool) (string, error) {


When writing the kubeconfig for the new EKS cluster then the following will happen: - If --kubeconfig, --auto-kubeconfig or KUBECONFING env var are NOT set then the default kube config location will be used (~/.kube/conf). - If KUBECONFIG is set then we use this (assuming --kubeconfig and --auto-kubeconfig aren't specified). - if KUBECONFIG is set BUT --kubeconfig or --auto-kubeconfig are specified then KUBECONFIG will be ignored. - if a kubeconfig file already exists then we merge in the new EKS cluster details with the existing file. - If `set-context` is true then the current-context will be set to that of the new EKS cluster. - If `set-context` is false (default) then if we are merging with an existing configuration file with a current-context set then this won't be updated. - If there is no existing configuration file then current-context will be set even if `set-context` is false. Issue #29

A number of changes in relation to the kube configuration file. The setting of current-context defaults to true. Also updated the write-kubeconfig command so that its merges configs as well. Also, when deleting a cluster it will only try and delete an auto-generated config file (otherwise it will warn that you need to do this manually). Issue #29

- rename kubeconf to kubeconfig - use default kubeconfig path directly - remove unecessary method for default path and use client-go instead - other minor cosmetic tweaks

The kubeconfig package has been modified to use `NewDefaultPathOptions` `ModifyConfig` like kops instead of other handcrafted methods. The plan will eventually to be re-use KubeConfBuilder from kops when it supports exec. Issue #29

The method to write the kube configuration file in the `kubeconfig` package has been change from `WriteKubeCfg` to `Write` as its simpler and more readable.

- set `--auto-kubeconfig=false` by default - fix default `--kubeconfig` value - pass context to command checker - do not pass `setContext` to client builder - obtain effective filename - remove unused import - improve output messages - use more explicit flag syntax in output message - add missing message in `eksctl utils write-kubeconfig` - clarify kubectl test warning

Add support for creating encrypted volume

errordeveloper mentioned this pull request Jun 22, 2018

Support merging of config files and KUBECONFIG #66

Closed

errordeveloper force-pushed the issue-29-merge-kubeconfig branch 2 times, most recently from 04425e9 to 40eafbd Compare June 22, 2018 17:04

errordeveloper commented Jul 3, 2018

View reviewed changes

errordeveloper force-pushed the issue-29-merge-kubeconfig branch from c721dd0 to 7639670 Compare July 4, 2018 16:06

errordeveloper changed the title ~~Support merging of Kubeconfig and KUBECONFIG~~ Extend kubeconfig writer to support global path with merging Jul 4, 2018

richardcase reviewed Jul 4, 2018

View reviewed changes

richardcase previously approved these changes Jul 4, 2018

View reviewed changes

errordeveloper dismissed richardcase’s stale review via 960940a July 4, 2018 21:06

errordeveloper force-pushed the issue-29-merge-kubeconfig branch 2 times, most recently from 960940a to 5381234 Compare July 5, 2018 04:47

richardcase and others added 6 commits July 5, 2018 06:29

More consistent naming, refactor utils and kubeconfig packages

ff443f7

- rename kubeconf to kubeconfig - use default kubeconfig path directly - remove unecessary method for default path and use client-go instead - other minor cosmetic tweaks

Simplify Makefile, run test during image build

573fd11

Updated to use clientcmd methods

fbef903

The kubeconfig package has been modified to use `NewDefaultPathOptions` `ModifyConfig` like kops instead of other handcrafted methods. The plan will eventually to be re-use KubeConfBuilder from kops when it supports exec. Issue #29

Change WriteKubeCfg to Write

08cb73c

The method to write the kube configuration file in the `kubeconfig` package has been change from `WriteKubeCfg` to `Write` as its simpler and more readable.

errordeveloper force-pushed the issue-29-merge-kubeconfig branch from b5f13fa to e4aeef5 Compare July 5, 2018 05:31

errordeveloper force-pushed the issue-29-merge-kubeconfig branch from e4aeef5 to 7e6bf07 Compare July 5, 2018 06:22

errordeveloper merged commit 1e2726a into master Jul 5, 2018

errordeveloper deleted the issue-29-merge-kubeconfig branch July 5, 2018 07:29

This was referenced Jan 26, 2019

Creating cluster in existing VPC times out waiting for node to join cluster #472

Closed

Create cluster in existing VPC fails with VPC CIDR block "192.168.0.0/16" is not the same as "172.31.0.0/16" #489

Closed

martina-if mentioned this pull request May 26, 2020

Provide a mechanism in eksctl to deploy a fully private EKS/VPC environment #1765

Closed

torredil pushed a commit to torredil/eksctl that referenced this pull request May 20, 2022

Merge pull request eksctl-io#80 from leakingtapan/kms

87e2ece

Add support for creating encrypted volume

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend kubeconfig writer to support global path with merging #80

Extend kubeconfig writer to support global path with merging #80

errordeveloper commented Jun 22, 2018

errordeveloper commented Jun 22, 2018 •

edited

Loading

errordeveloper commented Jun 23, 2018

richardcase commented Jun 25, 2018

richardcase commented Jun 25, 2018 •

edited

Loading

errordeveloper commented Jun 25, 2018

richardcase commented Jun 25, 2018

richardcase commented Jun 30, 2018

errordeveloper commented Jul 3, 2018

This comment was marked as abuse.

This comment was marked as abuse.

errordeveloper commented Jul 4, 2018

errordeveloper commented Jul 4, 2018

richardcase left a comment

This comment was marked as abuse.

This comment was marked as abuse.

Extend kubeconfig writer to support global path with merging #80

Extend kubeconfig writer to support global path with merging #80

Conversation

errordeveloper commented Jun 22, 2018

errordeveloper commented Jun 22, 2018 • edited Loading

errordeveloper commented Jun 23, 2018

richardcase commented Jun 25, 2018

richardcase commented Jun 25, 2018 • edited Loading

errordeveloper commented Jun 25, 2018

richardcase commented Jun 25, 2018

richardcase commented Jun 30, 2018

errordeveloper commented Jul 3, 2018

This comment was marked as abuse.

This comment was marked as abuse.

errordeveloper commented Jul 4, 2018

errordeveloper commented Jul 4, 2018

richardcase left a comment

Choose a reason for hiding this comment

This comment was marked as abuse.

This comment was marked as abuse.

errordeveloper commented Jun 22, 2018 •

edited

Loading

richardcase commented Jun 25, 2018 •

edited

Loading