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.

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 merging of config files and KUBECONFIG #66

Closed
wants to merge 2 commits into from

Conversation

richardcase
Copy link
Contributor

@richardcase richardcase commented Jun 17, 2018

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 true (default) then if we are merging with an
    existing configuration file with a current-context set then this won't
    be updated.

Issue #29

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 eksctl-io#29
@richardcase
Copy link
Contributor Author

Errors on further testing, closing.

@errordeveloper
Copy link
Contributor

@richardcase did you have any chance to resolve the issues? I'd love to get this in before beta.1 (aiming to cut it before end of the week or early next week), otherwise we might need to leave this until after 0.1.0.

@richardcase
Copy link
Contributor Author

@errordeveloper The merging of the config file and looking at the KUBECONFIG file all works. But when i was testing it i've discovered an issue with create cluster. Looking into it this is actually a problem related to the changes i made for #57. Its looking like a problem with using the --profile flag and the profile not passed to heptio authenticator when we generate a token so that it can then create the config map.

@errordeveloper
Copy link
Contributor

@richardcase ah, I see, that sounds like a bug and doesn't surprise me, as authenticator creates a separate client. I previously use AWS_PROFILE in all of my tests, and that obviously worked. I think we should re-open and review this, and fix the authenticator bug separately.

@richardcase
Copy link
Contributor Author

@errordeveloper - sounds like a plan. I have a few changes that i need to include in this. I'll get them committed and pushed.

@errordeveloper
Copy link
Contributor

errordeveloper commented Jun 19, 2018 via email

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 eksctl-io#29
@richardcase
Copy link
Contributor Author

@errordeveloper - it should be good for testing.

}
}

func writeConfig(filename string) error {

This comment was marked as abuse.

@errordeveloper
Copy link
Contributor

@richardcase I've added some more changes to this, and figured it'll be easier to review as one PR – please see #80.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants