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

Replacing RunnerPodSpec with corev1.PodSpec #903

Closed
wants to merge 1 commit into from

Conversation

luizbafilho
Copy link
Contributor

@luizbafilho luizbafilho commented Aug 28, 2023

fixes #786

To allow full customization of the runner pod, this PR replaces the custom RunnerPodSpec type with corev1.PodSpec, and uses the mergo package to merge the default settings with the user's ones.

@luizbafilho luizbafilho changed the title Replacing RunnerPodSpec with corev1.PodSpec to allow full user custom… Replacing RunnerPodSpec with corev1.PodSpec Aug 29, 2023
@luizbafilho luizbafilho requested a review from squaremo August 29, 2023 16:15
@squaremo
Copy link
Contributor

I understand the motivation, and I can see the problem-- PodSpec has required fields, and as soon as you are supplying a PodSpec value you will have to provide all of those. What you need is PodSpec but with everything marked optional, and I can think of some ways of getting that:

  1. make a copy of the PodSpec type locally and change all the annotations
  2. patch the generated CRD schema so that the fields are all optional
  3. use a freeform-JSON type for the field

All of these have obvious problems. Patching the schema might have the least serious problems, since schema changes can be caught during build.

@squaremo
Copy link
Contributor

  1. If it's not a requirement to be able to patch anything in the pod spec, then just including the fields known to be required would be fine.

@yiannistri
Copy link
Contributor

Closing in favour of #910

@yiannistri yiannistri closed this Sep 4, 2023
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.

Ability to set priorityClass on tf-runner pod
3 participants