DM-43555: Add phalanx environment install command #3165
Conversation
rra
force-pushed
the
tickets/DM-43555
branch
11 times, most recently
from
3b6d7bc
to
fac80f8
Compare
rra
force-pushed
the
tickets/DM-43555
branch
2 times, most recently
from
9d578cb
to
6652978
Compare
There was a problem hiding this comment.
LGTM.
I have some questions about "can we make this a shared library" around the Command class mostly, and...while I am certainly not someone who should throw stones about just shelling out via Command or its moral equivalent (git versus GitPython in particular), your kubectl driver feels like, except for waiting for rollouts, it's stuff we already have in Safir.
| if not origin.url: | ||
| raise GitRemoteError('Remote "origin" has no URL') | ||
|
|
||
| # If the URL is not an https URL, accept a few forms of github.com |
There was a problem hiding this comment.
Nice. Also something that probably ought to be lifted into a shared library.
|
|
||
| # Prompt the user unless they specifically said not to. | ||
| if not force_noninteractive: | ||
| print(_INSTALL_WARNING.format(environment=environment)) |
There was a problem hiding this comment.
I don't know if click.confirm will let you do this, and maybe it's being TOO aggressive...
...but given my tendency to speed through installers without thinking about them, I feel like it might be a good idea to generate a nonce -- maybe the classical six-digit random PIN -- display it, and make the user type it or at least copy-paste it, rather than just "y-Return".
There was a problem hiding this comment.
I'm going to merge this in its current form, but I added to my to-do list determining the current configured Kubernetes cluster and displaying that as part of the prompt to make it easier for people to realize when they're pointing to the wrong cluster.
| __all__ = ["KubernetesStorage"] | ||
|
|
||
|
|
||
| class KubernetesStorage: |
There was a problem hiding this comment.
On the one hand, simpler...but on the other hand it feels like we already HAVE a lot of it mocked out for testing if we were using the Safir driver like we do in the Nublado controller. I mean...we already have secrets and namespaces in there, right?
There was a problem hiding this comment.
Yes, the thing we don't have is waiting for a rollout to complete. I don't think that's a trivial thing to implement with timeout and error handling, etc., and we're doing everything else with command-line tools (Helm and Argo CD), so it didn't seem worth the effort to reimplement it in Python.
The testing code isn't that useful for Phalanx since Phalanx already tests the installer with a good integration test, making unit tests (particularly of this stuff) fairly pointless.
Port the installer/install.sh script to Python as the new phalanx environment install command. Add a strongly worded confirmation message before proceeding, which can be overrriden with a command line flag for CI testing. Expand the Helm storage layer and add Argo CD and Kubernetes storage layers to support the installer. Use kubectl rather than the Python Kubernetes libraries to manipulate Kubernetes objects, since it's simpler and more straightforward for the type of actions the installer needs to take. Introduce a new way of passing around Vault credentials, and support authenticating to Vault with an AppRole, at least in the context of the installer. (The secrets and vault commands still require tokens.) Remove the old installer and switch GitHub Actions CI over to the new command.
Remove documentation for the old install script and document the new installation process. Fix an ordering problem in the documentation for how to set up a new environment by putting assembly of the configuration for that environment before setting up secrets management.
Be a bit clearer about how the secret for vault-secrets-operator is created.
Add better explanations of the GitHub-specific output code and the command execution layer.
Wait for a minute instead of 30 seconds for the sync of each infrastructure application.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
When running in the merge queue, we do fall back on Git operations to determine the current branch. Return the branch name, not a Head object.
We're still getting timeouts in GitHub Actions. Increase the timeouts for Helm and Argo CD operations even further.
Port the installer/install.sh script to Python as the new phalanx environment install command. Add a strongly worded confirmation message before proceeding, which can be overrriden with a command line flag for CI testing.
Expand the Helm storage layer and add Argo CD and Kubernetes storage layers to support the installer. Use kubectl rather than the Python Kubernetes libraries to manipulate Kubernetes objects, since it's simpler and more straightforward for the type of actions the installer needs to take.
Introduce a new way of passing around Vault credentials, and support authenticating to Vault with an AppRole, at least in the context of the installer. (The secrets and vault commands still require tokens.)
Remove the old installer and switch GitHub Actions CI over to the new command.