remote-run for toolbox containers #4042
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Qmando
reviewed
Apr 14, 2025
Comment on lines
+152
to
157
| instance_or_toolbox.add_argument( | ||
| "--toolbox", | ||
| help="The selected service is a 'toolbox' container", | ||
| action="store_true", | ||
| default=False, | ||
| ) |
There was a problem hiding this comment.
Could we leave this out and derive it entirely from the service arg?
There was a problem hiding this comment.
We could, but I usually prefer to be explicit with this stuff. Ensures that the default behaviour will always be to launch a service pod, without relying on a naming convention.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Forgive me PaaSTA gods for the sins I'm about to commit.
Hi, I know this is a bit... hacky, but it's the best way I could think to reuse the same implementation. If I had to describe the change with one image, it'd be something like:
So, to explain a bit, the plan which was discussed internally as result of the CEP is to re-use the same remote-run logic to launch "toolbox container" which would give access to selected CLI tools to developers. We don't want these images to be deployed like mock services, as that would be scope creep of what's supposed to be in soa-configs, and we only require some small set of startup settings anyways. So the plan would be to define those settings as part of the paasta system config, and generate a deployment configuration on the fly from them, which is exactly what this change does.
I tested the generation of a K8s Job spec from this assembled configuration, and it looks reasonable to me. The one thing will lose meaning is the "git_sha" field, as that's strictly related to how paasta actually marks deployments, but I don't think that would create problems.
I had to just make a couple of adjustments to the main deployment logic:
The final notable difference is that, since we set up sshd in these pods, we don't need to
kubectl execinto them, but just grab the pod IP as part of the status polling response, and ssh directly into them. This choice was done since it makes it way easier to audit user actions inside the containers.