Skip to content
This repository has been archived by the owner on May 7, 2024. It is now read-only.

Adapt 'gsctl create kubeconfig' to add support for v5 clusters #437

Merged
merged 7 commits into from
Oct 4, 2019

Conversation

marians
Copy link
Member

@marians marians commented Sep 30, 2019

Towards https://github.com/giantswarm/giantswarm/issues/6447, https://github.com/giantswarm/giantswarm/issues/5465

This PR changes the behaviour if the gsctl create kubeconfig command

  • to support v5 clusters, as it attempts to fetch v5 cluster details for the given cluster ID first, then only attempts v4 if that failed.
  • some output regarding the crteated files is only printed when the -v/--verbose flag is applied, to make the output cleaner.

Preview

image

@marians marians self-assigned this Sep 30, 2019
Copy link
Contributor

@tuommaki tuommaki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with this in general. One note about v5 vs. v4 logic, but if necessary, that could be also addressed in separate PR. Either way.

apiEndpoint = fmt.Sprintf("https://%s.%s", tenantInternalAPIPrefix, strings.Join(baseEndpoint, urlDelimiter))
} else {
apiEndpoint = clusterDetailsResponse.Payload.APIEndpoint
if result.apiEndpoint == "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to extract this to separate function and inline the call in if-case above? I find this logic a little difficult to follow, because first it works with V5, then if that fails it potentially just logs and next step is suddenly v4 call if some variable is unset so far.

I feel like these generated types require some wrapping so that the higher level business logic would be clean, but is a bit overkill at this point maybe.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed the way cluster details are pulled.

@coveralls
Copy link

coveralls commented Oct 2, 2019

Coverage Status

Coverage decreased (-0.07%) to 42.994% when pulling 8bdd768 on create-kubeconfig-v5 into b03eddf on master.

@marians marians merged commit f395d32 into master Oct 4, 2019
@marians marians deleted the create-kubeconfig-v5 branch October 4, 2019 14:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants