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

Adds support for contextNames in tk env subcommands #704

Merged
merged 3 commits into from
Jun 2, 2022

Conversation

Nashluffy
Copy link
Contributor

@Nashluffy Nashluffy commented May 17, 2022

This compliments #674 and probably should have been in there to begin with.

This PR adds support for contextNames using tk env add and tk env set commands. It leverages the Connect method to check validity of the resulting config.

Below are a few examples, including cases that would throw errors

(~/Code) Nash $ mkdir example-tk && cd example-tk
(~/Code/example-tk) Nash $ tk init
GET https://github.com/jsonnet-libs/k8s-libsonnet/archive/1942ec15993b5a273569bb9a59d522b01522d0db.tar.gz 200
GET https://github.com/grafana/jsonnet-libs/archive/73396f2821466dd8f91181e7ee4513e0a8ea45b3.tar.gz 200
Directory structure set up! Remember to configure the API endpoint:
`tk env set environments/default --server=https://127.0.0.1:6443`


(~/Code/example-tk) Nash $ tk env set environments/default --context-name "devwat-us-south.*" --server localhost --namespace default
updated spec.apiServer (`` -> `localhost`)
updated spec.contextNames (`[]` -> `[devwat-us-south.*]`)
Error: Your Environment's spec.json seems incomplete:
  * spec.apiServer|spec.contextNames: These fields are mutually exclusive, please only specify one.

Please see https://tanka.dev/config for reference

(~/Code/example-tk) Nash $ tk env set environments/default --context-name "devwat-us-south.*" --namespace default
updated spec.contextNames (`[]` -> `[devwat-us-south.*]`)
(~/Code/example-tk) Nash $ cat environments/default/spec.json
{
  "apiVersion": "tanka.dev/v1alpha1",
  "kind": "Environment",
  "metadata": {
    "name": "environments/default",
    "namespace": "environments/default/main.jsonnet"
  },
  "spec": {
    "apiServer": "",
    "contextNames": [
      "devwat-us-south.*"
    ],
    "namespace": "default",
    "resourceDefaults": {},
    "expectVersions": {}
  }
}

(~/Code/example-tk) Nash $ tk env set environments/default --server localhost --namespace default
updated spec.apiServer (`` -> `localhost`)
Error: Your Environment's spec.json seems incomplete:
  * spec.apiServer|spec.contextNames: These fields are mutually exclusive, please only specify one.

Please see https://tanka.dev/config for reference

(~/Code/example-tk) Nash $ tk env add example --context-name "devwat-us-south.*"
(~/Code/example-tk) Nash $ cat example/spec.json
{
  "apiVersion": "tanka.dev/v1alpha1",
  "kind": "Environment",
  "metadata": {
    "name": "example"
  },
  "spec": {
    "apiServer": "",
    "contextNames": [
      "devwat-us-south.*"
    ],
    "namespace": "default",
    "resourceDefaults": {},
    "expectVersions": {}
  }
}
(~/Code/example-tk) Nash $

@malcolmholmes
Copy link
Contributor

There is good reason why context name is not used in spec.json: it can vary across installations, meaning different users get differing results.

What could work would be allowing a tk env add command that uses a context name, but writes the apiServer URL into the spec.json instead. That way, initial user experience is improved (I can specify the cluster using a term I'm familiar with) yet consistency is maintained (every user references the same api server).

@Nashluffy
Copy link
Contributor Author

@malcolmholmes support for context names in spec.json was added in v0.21.0 release of Tanka (introduced in this pull request). This is just adding the corresponding functionality to tk env commands.

@malcolmholmes
Copy link
Contributor

@Nashluffy I'm clearly out of date!

Copy link
Member

@julienduchesne julienduchesne left a comment

Choose a reason for hiding this comment

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

LGTM! Makes sense that if it's supported in the spec.json, it should be in these commands

@julienduchesne julienduchesne merged commit ec10893 into grafana:main Jun 2, 2022
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.

3 participants