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

add tolerations/nodeselector to migration job template (fixes #1774) #1804

Merged
merged 1 commit into from
May 1, 2024

Conversation

ranvit
Copy link
Contributor

@ranvit ranvit commented Apr 2, 2024

SUMMARY

As described in #1774, the migration pod has no provisions to add tolerations to it. This PR's solution is to use the postgres pod's toleration/selector on the migration job.

The docs would need to be updated to alert the user about this relationship between postgres and the migration pod

ISSUE TYPE
  • Bug, Docs Fix or other nominal change

@rooftopcellist
Copy link
Member

@ranvit Is there a reason why postgres_tolerations and postgres_node_selector is used instead of task_tolerations and task_node_selector?

My thinking here is that specifying postgres_ params might not make sense in some scenarios, for example external db scenarios.

The k8s db migration job uses the task container, and previously this was running in the task container, so using task_* params for this might be a bit more intuitive.

Also, could you add some docs here to make this a bit more discoverable for users?

…nsible#1804)

Modified the db-migration job template to use `task_*` settings with a fallback to global AWX configurations if not specified.
@ranvit ranvit force-pushed the migration-job-toleration branch from 7e98f78 to cb3dcc6 Compare April 29, 2024 18:24
@ranvit
Copy link
Contributor Author

ranvit commented Apr 29, 2024

@rooftopcellist thank you for the feedback. It makes sense for this migration job to utilize the task_* settings.

I decided to implement this the same way the Task pods have been configured here so that we get the benefit of falling back to the global node assignment settings

Also updated the docs!

Copy link
Member

@rooftopcellist rooftopcellist left a comment

Choose a reason for hiding this comment

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

I tested this out on my cluster and it works as expected as far as I can tell. No errors on fresh install either.

@ranvit Thanks for the PR!

@rooftopcellist rooftopcellist merged commit 4fc20de into ansible:devel May 1, 2024
7 checks passed
@ranvit
Copy link
Contributor Author

ranvit commented May 7, 2024

@rooftopcellist how do you feel about adding the same tolerations/nodeSelector fallback on the Backup role's db-management pod?

I could go through every k8s pod template in the repo and add the same fix

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

Successfully merging this pull request may close these issues.

2 participants