Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

Use hub in after inits of multi cluster environments #566

Merged
merged 3 commits into from
Nov 19, 2018

Conversation

alljames
Copy link
Contributor

@alljames alljames commented Oct 3, 2018

What this PR does / why we need it:
Fixes

  1. tarmak init panics when run against empty ~/.tarmak
  2. when a multicluster environment is initialised, the Tarmak CLI points at cluster instead of hub even though user will have to switch context to hub. This PR ensures that the default is for the CLI to point to the hub.

Which issue this PR fixes fixes #556

Release note:

If the multicluster environment being initialised is the only existing environment, Tarmak CLI now points to that environment's 'hub' cluster

@jetstack-bot jetstack-bot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Oct 3, 2018
@jetstack-bot jetstack-bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 3, 2018
@simonswine simonswine changed the title Improve behaviour of tarmak init Use hub in after inits of multi cluster environments Oct 23, 2018
if a.tarmak.Cluster() != nil {
if err := a.verifyInstanceTypes(); err != nil {
result = multierror.Append(result, err)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can get rid of this and discuss it in #559

@simonswine
Copy link
Contributor

/assign @alljames
/milestone release-0.5

@simonswine simonswine added this to the release-0.5 milestone Oct 23, 2018
@jetstack-bot jetstack-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 23, 2018
Copy link
Contributor

@simonswine simonswine left a comment

Choose a reason for hiding this comment

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

Just a quick review, would be good to get that into 0.5.0

// ensure that the tarmak cli is pointing to that environment's 'hub' cluster
// resource, else set point tarmak cli to newly-initialised cluster
switch cluster.Type() {
case "cluster-multi":
Copy link
Contributor

Choose a reason for hiding this comment

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

we should use a constant here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now uses a const

pkg/tarmak/tarmak.go Show resolved Hide resolved
… default on new multicluster

Signed-off-by: Aled James <aledjms@gmail.com>
Signed-off-by: Aled James <aledjms@gmail.com>
Signed-off-by: Aled James <aledjms@gmail.com>
@alljames
Copy link
Contributor Author

/unassign
/assign @simonswine

@jetstack-bot jetstack-bot assigned simonswine and unassigned alljames Nov 12, 2018
@simonswine
Copy link
Contributor

/approve
/lgtm

@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 19, 2018
@jetstack-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: simonswine

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jetstack-bot jetstack-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 19, 2018
@jetstack-bot jetstack-bot merged commit 0788f8d into jetstack:master Nov 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tarmak init for new multi cluster environments should set the hub as the current cluster
3 participants