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

[ClusterQueue] Provide more details about the mis-configuration in the message for Active=False #3099

Open
3 tasks
mimowo opened this issue Sep 19, 2024 · 7 comments
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@mimowo
Copy link
Contributor

mimowo commented Sep 19, 2024

What would you like to be added:

More details in the message for the "Active=False" condition for ClusterQueue in case of mis-configuration.

Currently we have the reason field determined here:

func (c *clusterQueue) inactiveReason() (string, string) {
, but the message remains generic "Can't admit new workloads."

Why is this needed:

  • To phrase the configuration issue in a more human-readable way, I find "FlavorIndependentAdmissionCheckAppliedPerFlavor" is hard to understand
  • To provide more details which AdmissionCheck or ResourceFlavor is mis-configured, which can be handy when there are more than one.

Completion requirements:

This enhancement requires the following artifacts:

  • Design doc
  • API change
  • Docs update

The artifacts should be linked in subsequent comments.

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

mimowo commented Sep 19, 2024

I believe it would be nice to complete this before #3095.

/cc @alculquicondor @tenzen-y @trasc

@alculquicondor
Copy link
Contributor

+1

@trasc
Copy link
Contributor

trasc commented Sep 19, 2024

Based on the implementation the massage should already contain a concatenation of all the reasons:

return reasons[0], strings.Join([]string{"Can't admit new workloads:", strings.Join(reasons, ", ")}, " ")

@tenzen-y
Copy link
Member

+1

Granular messages would always be helpful for actual cluster operation.

@trasc
Copy link
Contributor

trasc commented Sep 20, 2024

The message can be made more human readable and maybe include the missing flavor name, AC name and so on. Trimming it should not be necessary since it can hold 32K.

@trasc
Copy link
Contributor

trasc commented Sep 20, 2024

/assign

@trasc
Copy link
Contributor

trasc commented Sep 20, 2024

/retitle [ClusterQueue] Provide more details about the mis-configuration in the message for Active=False

@k8s-ci-robot k8s-ci-robot changed the title [MultiKueue] Provide more details about the mis-configuration in the message for Active=False [ClusterQueue] Provide more details about the mis-configuration in the message for Active=False Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

4 participants