Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First wave of fixes for remote-run #4033

Merged
merged 4 commits into from
Mar 27, 2025
Merged

First wave of fixes for remote-run #4033

merged 4 commits into from
Mar 27, 2025

Conversation

piax93
Copy link
Contributor

@piax93 piax93 commented Mar 26, 2025

  • Describe 403 code for remote-run apis
    • Since OPA can deny remote-run requests, it'd be nice to have the right code returned from the API server rather than 500 cause the code is not in the spec (this was actually raise by @Qmando in remote-run api implementation #4022 but I turned the suggestion down, mistakenly, sorry)
  • Add more output to remote-run cli
    • ... because it's way too silent now, and pod don't come online instantaneusly
  • Fix wrong kwarg in paasta_cleanup_remote_run_resources
    • This is just linters failing me (and too much mocking in unit tests)
  • Drop unneeded pod options for remote-run jobs
    • We do not want this to become a way for people to deploy services in prod, so in general these pods should not have routable IPs (it'll be a different story for toolbox containers as those will run sshd).
    • In my recent testing the job pod was failing to becomes ready due to hacheck. I think that may be due to some zookeeper locking which interfered with the existing replicas of the service, but at any rate, we don't want that, or any other sidecar, as this is effectively meant to be "local-run, but in a pod"

@piax93 piax93 requested a review from a team as a code owner March 26, 2025 16:52
Comment on lines +2228 to +2231
has_routable_ip = (
"false"
if force_no_routable_ip
else self.has_routable_ip(service_namespace_config, system_paasta_config)
Copy link
Member

Choose a reason for hiding this comment

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

one day we'll update things so that the default is no routable ip #dream

@nemacysts nemacysts merged commit d05f3b4 into master Mar 27, 2025
10 checks passed
@piax93 piax93 deleted the u/mpiano/SEC-19951 branch March 27, 2025 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants