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

internal/server: refactor install into one package #882

Merged
merged 16 commits into from
Dec 4, 2020

Conversation

krantzinator
Copy link
Contributor

@krantzinator krantzinator commented Dec 4, 2020

Foundation to addressing #672 and #612
There's a lot of diff from moving existing code around, so I'll highlight actual changes here:

  • Moved the platforms to implement an Installer interface, replacing the case setup in InstallCommand to set the platform
  • Took serverinstall.Config out of InstallCommand and gave each Platform its own config
    • This was the main place I went in circles with; I wanted each Platform to have its own config, and only care about the flags it needed to. I tried out a separate serverinstall/config package; toyed with using serverconfig; moved the InstallFlags pieces from InstallCommand to serverinstall to separate packages and back again. I also worked on this piece with a couple teammates, experimenting with different ways to have generic flags and install them to the scoped platforms as part of init. Ultimately I decided to go with the Platform-specific flags you see here in order to overcome the various hurdles discovered.

Small pieces:

  • fixed a bug in the k8s statefulset; we weren't actually using the value set in the pull-policy flag, and updated the flag to have an updated value of IfNotPresent to match k8s default (ImagePullPolicy: apiv1.PullPolicy(c.ImagePullPolicy),
  • threw a strings.ToLower on the platform name for UX

Copying Evan's example of including examples in lieu of a CLI testing framework:

$ waypoint-dev install -platform=nomad -nomad-dc=dc1 -accept-tos
✓ Nomad allocation ready
✓ Configuring server...
Waypoint server successfully installed and configured!
...

$ waypoint-dev install -platform=docker -accept-tos
✓ Installing Waypoint server to docker
✓ Server container started!
✓ Configuring server...
Waypoint server successfully installed and configured!
...

$ waypoint-dev install -platform=kubernetes -accept-tos
✓ Creating Kubernetes resources...
✓ Kubernetes StatefulSet reporting ready
✓ Waiting for Kubernetes service to become ready..
✓ Configuring server...
Waypoint server successfully installed and configured!
...

Copy link
Contributor

@mitchellh mitchellh left a comment

Choose a reason for hiding this comment

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

Yeahhhhhhh this is way cleaner.

Just a couple tiny comments but this is good work.

internal/serverinstall/docker.go Outdated Show resolved Hide resolved
internal/serverinstall/serverinstall.go Show resolved Hide resolved
internal/serverinstall/serverinstall.go Outdated Show resolved Hide resolved
Copy link
Member

@briancain briancain left a comment

Choose a reason for hiding this comment

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

📦 🎉

@krantzinator krantzinator merged commit f9def34 into main Dec 4, 2020
@krantzinator krantzinator deleted the f-uninstall-refactor branch December 4, 2020 22:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants