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

runner install profile fixes & improvement #3755

Merged
merged 7 commits into from
Aug 30, 2022

Conversation

paladin-devops
Copy link
Contributor

[ ] Fix the ODR config for profiles created during runner install to not set the new profile as the default
[ ] Fix the ODR config for profiles created during runner install to include the runner ID in the profile name
[ ] Modify behavior of runner install to set target labels instead of target ID on the runner profile if the user provided one or more -label flags after -- during runner install. Example below :

$ waypoint runner install -platform=kubernetes -server-addr=localhost:9701 -server-tls-skip-verify -- -label=hello=paladin -label world=devops
// Successful install

$ waypoint runner profile list
Runner profiles
                  NAME                  | PLUGIN TYPE |            OCI URL            |                TARGET RUNNER                 | DEFAULT
----------------------------------------+-------------+-------------------------------+----------------------------------------------+----------
 
  kubernetes-01GBNJDBGHP7KAACB91HHPHJ0K | kubernetes  | hashicorp/waypoint:latest     | labels: {"hello":"paladin","world":"devops"} | 

@krantzinator
Copy link
Contributor

krantzinator commented Aug 29, 2022

Edit: I am now realizing this is not related to your PR, exactly, so lmk if I should open an issue or we can discuss separately!
Looks great! Quick question about the name we generate for the static runner profile -- isn't the static runner's ID static?

❯ wd runner profile list                                                                                                                                                 17:10:17
Runner profiles
                 NAME                  | PLUGIN TYPE |                OCI URL                 |            TARGET RUNNER            | DEFAULT
----------------------------------------+-------------+----------------------------------------+-------------------------------------+----------
 01GBNSDX79V1T8K13DSQEXBT9C            | kubernetes  | docker.io/hashicorp/waypoint-odr:0.9.1 | *                                   | yes
 kubernetes-bootstrap-profile          | kubernetes  | hashicorp/waypoint-odr:latest          | *                                   | yes

Also, why does that OCI URL list docker.io and the others -- including those resulting from runner install commands -- not include the docker.io/ prefix?

Copy link
Contributor

@krantzinator krantzinator left a comment

Choose a reason for hiding this comment

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

Looks fantastic, and working well for me!

@paladin-devops
Copy link
Contributor Author

Quick question about the name we generate for the static runner profile -- isn't the static runner's ID static?

It is indeed static! However for the installation of the static runner with the server, we decided the profile would be given a name with the convention <PLUGIN_TYPE>-bootstrap-profile when the server install code was updated to work in conjunction with the changes for runner install.

Also, why does that OCI URL list docker.io and the others -- including those resulting from runner install commands -- not include the docker.io/ prefix?

This is because of the package we're using to parse the repository name for the Docker image supplied to Kubernetes for runner install, github.com/novln/docker-parser in line 148 of internal/runnerinstall/k8s.go (below). The repository and image name are parsed from the tag because they are two separate values in the Helm chart. docker.io is the default hostname, so that appears to be returned back to the caller after parsing.

odrImageRef, err := dockerparser.Parse(odrImage)

… runner profile as default. Also fix the config to include the ID in the name of the profile again.
This commit updates the OnDemandRunnerConfig interface to accept no arguments once again. The runner install CLI appends the ID of the runner to the profile name after this config is retrieved. Also, if target labels for the runner profile are specified (not possible yet), then the profile uses target labels instead of a target ID.
@krantzinator
Copy link
Contributor

Ah, just realized the docs need to be updated now! https://www.waypointproject.io/docs/runner/profiles#viewing-runner-profiles
At least, the example of what the default runner profiles look like.

Copy link
Contributor

@krantzinator krantzinator left a comment

Choose a reason for hiding this comment

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

Re-approve!

@paladin-devops paladin-devops added the backport/0.9.x Automerge PR into release/0.9.x branch after merge to main label Aug 30, 2022
@paladin-devops paladin-devops merged commit 4b93417 into main Aug 30, 2022
@paladin-devops paladin-devops deleted the b-runner-install-profile-fixes branch August 30, 2022 14:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/0.9.x Automerge PR into release/0.9.x branch after merge to main core website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants