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

validate_tests should not allow an exception to custom definitions to have status #1386

Closed
jlewi opened this issue Jul 14, 2020 · 2 comments · Fixed by #1389
Closed

validate_tests should not allow an exception to custom definitions to have status #1386

jlewi opened this issue Jul 14, 2020 · 2 comments · Fixed by #1389
Labels

Comments

@jlewi
Copy link
Contributor

jlewi commented Jul 14, 2020

#1375 added an exception to our unittest to ensure resources don't have status fields to allow customresourcedefinitions to have
status.

if m.Kind == "CustomResourceDefinition" {

I don't think we want to allow status fields.

The original motivation for this test #1179 was to support git ops tools. Having status fields in resources is problematic for gitops tools because (one proposal) for the semantic are

In general, what is declared in git is "the subset of fields and values that my objects must have set." Other fields and values can be present, but they can not conflict. Empty status is problematic because an empty status in git is essentially declaring "I always want an empty status on this resource." Beyond that, declaring a status in the source of truth is wrong from a semantic point of view.

These are the semantics expressed by ACM at least. Nonetheless, I think it is consistent with the K8s pattern that you should only be updating and specifying the spec.

It looks like the problem might be that earlier versions of kubebuilder were including CRD's in the status.

I think its better if we handle the cleanup of status in kubeflow/manifests and don't force users to have to deal with it.

The unittest is intended to capture examples that break this (e.g. the Seldon CRD) and then fix it. There are a couple approaches to fixing it

  1. Application owners can choose to manually edit the CRDs or other resources and remove status
  2. We can invest in writing a kustomize function to transform all resources and remove resources

Proposal

@cliveseldon does that seem reasonable?

@issue-label-bot
Copy link

Issue-Label Bot is automatically applying the labels:

Label Probability
kind/bug 0.67

Please mark this comment with 👍 or 👎 to give our bot feedback!
Links: app homepage, dashboard and code for this bot.

@ukclivecox
Copy link
Contributor

This looks like its due to kubernetes-sigs/controller-tools#456

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

Successfully merging a pull request may close this issue.

2 participants