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

serverconfig: store platform in server config #1000

Merged
merged 3 commits into from
Jan 21, 2021

Conversation

briancain
Copy link
Member

Prior to this commit, waypoint had no way internally to figure out what
kind of platform a server was, which meant it had to rely on the user to
pass the proper flag when doing any server operations on an existing
context. It also meant it was difficult to realize what a server
platform was when looking at existing context through waypoint context list. This commit updates that to store the context passed in from
install.

For any previous context prior to this change, we will show n/a in the
waypoint context list output since we did not store that context platform.

brian@localghost:go % waypoint context list                                                                                                                                                                                           ±[main]
    |        NAME        | PLATFORM | SERVER ADDRESS
----+--------------------+----------+-----------------
    | install-1611251878 | n/a      | localhost:9701
    | install-1611251992 | docker   | localhost:9701
  * | install-1611252817 | docker   | localhost:9701

Fixes #893

Prior to this commit, waypoint had no way internally to figure out what
kind of platform a server was, which meant it had to rely on the user to
pass the proper flag when doing any server operations on an existing
context. It also meant it was difficult to realize what a server
platform was when looking at existing context through `waypoint context
list`. This commit updates that to store the context passed in from
install.
@briancain briancain requested a review from a team January 21, 2021 18:20
@github-actions github-actions bot added the core label Jan 21, 2021
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.

This PR is good as-is. Is the plan to use this for upgrade and uninstall then to not require the -platform flag?

internal/server/proto/server.proto Show resolved Hide resolved
@briancain
Copy link
Member Author

@mitchellh - Yes! Eventually I think we can make it so that the platform flag won't be required for those commands, but we'll need users to use it at least for a while since old contexts won't have platform stored.

@briancain
Copy link
Member Author

Also I was thinking the upgrade/uninstall enhancement to use this can come after the initial PRs go through since it's separate.

@mitchellh
Copy link
Contributor

Also I was thinking the upgrade/uninstall enhancement to use this can come after the initial PRs go through since it's separate.

Yes definitely

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.

🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Platform as a serverconfig option
3 participants