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

Make LossLessDefaulter selective to only prevent dropping fields outside of schema #3174

Closed
Tracked by #3192
mimowo opened this issue Oct 1, 2024 · 6 comments
Closed
Tracked by #3192
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@mimowo
Copy link
Contributor

mimowo commented Oct 1, 2024

What would you like to be added:

Investigate making the LossLessDefaulter in Kueue to only prevent dropping fields outside of schema.
This is a follow up from the discussion: #3132 (comment)

And follow up to the issue: #2878

Why is this needed:

  • Long term we may want to remove some fields in Kueue webhooks, and it would be hard with the current LossLessDefaulter
  • Making LossLessDefaulter selective could possibly let us promote it as an opt-in defaulter in controller-runtime, allowing for:
    • reduced maintenance cost by moving from Kueue to controller-runtime
    • allow sharing the solution with other projects so that they don't need to reinvent the wheel

Completion requirements:

Investigate if this is feasible. and what is the implementation complexity, prototype.

@mimowo mimowo added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 1, 2024
@mimowo
Copy link
Contributor Author

mimowo commented Oct 1, 2024

/assign @tenzen-y
per comment #3132 (comment)
/cc @alculquicondor

@tenzen-y
Copy link
Member

tenzen-y commented Oct 1, 2024

Sorry. I did not want to indicate taking this issue.
/unassign

@mimowo
Copy link
Contributor Author

mimowo commented Oct 2, 2024

Sorry, I misread your message which was clear. Thanks for clarifying!
Anyway, the issue is open for contributions.

@mbobrovskyi
Copy link
Contributor

/assign

@mimowo
Copy link
Contributor Author

mimowo commented Nov 7, 2024

/close
we don't need this in Kueue as there is fix in controller-runtime to be released, issue to track using the fix: #3469

@k8s-ci-robot
Copy link
Contributor

@mimowo: Closing this issue.

In response to this:

/close
we don't need this in Kueue as there is fix in controller-runtime to be released, issue to track using the fix: #3469

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

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