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

✨ Remove status from generated CRDs #433

Merged
merged 1 commit into from
Apr 28, 2020

Conversation

mcristina422
Copy link
Contributor

@mcristina422 mcristina422 commented Apr 24, 2020

This sets the CRDs status to empty so it is no longer generated in CRD yamls.

This was originally added in #203 which confuses me because v1beta1 has had the optional marker for awhile https://github.com/kubernetes/kubernetes/pull/66102/files#diff-3029a18192feb70af52de676209da9b6R206

This is currently a problem because continuous delivery tools cant converge when applying them against the updated server set properties

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 24, 2020
@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Apr 24, 2020
@mcristina422
Copy link
Contributor Author

/assign @vincepri

@vincepri
Copy link
Member

/approve
/assign @alvaroaleman @DirectXMan12 @mengqiy

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 24, 2020
@mengqiy
Copy link
Member

mengqiy commented Apr 24, 2020

@DirectXMan12 Can you please confirm if it's now safe to do this?

@DirectXMan12
Copy link
Contributor

Can you quickly confirm that you've run this against a few different API server version and kubectl versions? If this serialized properly now (as shown in the PR) we should be fine, but better safe than sorry.

@DirectXMan12
Copy link
Contributor

mainly just looking for a slighly older version (e.g. 1.14 or something)

@mcristina422
Copy link
Contributor Author

Using cronjobs.testdata.kubebuilder.io as the example is problematic because the spec includes fields that do not pass kubectl's validation already.

The CustomResourceDefinition "cronjobs.testdata.kubebuilder.io" is invalid: spec.validation.openAPIV3Schema.properties[spec].properties[embeddedResource].properties: Required value: must not be empty if x-kubernetes-embedded-resource is true without x-kubernetes-preserve-unknown-fields

I generated a new CRD with simply

type CronJobSpec struct {
	Schedule string `json:"schedule"`
}
CRD YAML
---
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  annotations:
    controller-gen.kubebuilder.io/version: (devel)
  creationTimestamp: null
  name: cronjobs.testdata.kubebuilder.io
spec:
  group: testdata.kubebuilder.io
  names:
    kind: CronJob
    listKind: CronJobList
    plural: cronjobs
    singular: mycronjob
  scope: Namespaced
  versions:
  - name: v1
    schema:
      openAPIV3Schema:
        description: CronJob is the Schema for the cronjobs API
        properties:
          apiVersion:
            description: 'APIVersion defines the versioned schema of this representation
              of an object. Servers should convert recognized schemas to the latest
              internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#resources'
            type: string
          kind:
            description: 'Kind is a string value representing the REST resource this
              object represents. Servers may infer this from the endpoint the client
              submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#types-kinds'
            type: string
          metadata:
            type: object
          spec:
            properties:
              schedule:
                description: The schedule in Cron format, see https://en.wikipedia.org/wiki/Cron.
                type: string
            required:
            - schedule
            type: object
          status:
            description: CronJobStatus defines the observed state of CronJob
            properties:
              active:
                description: A list of pointers to currently running jobs.
                items:
                  description: ObjectReference contains enough information to let
                    you inspect or modify the referred object.
                  properties:
                    apiVersion:
                      description: API version of the referent.
                      type: string
                    fieldPath:
                      description: 'If referring to a piece of an object instead of
                        an entire object, this string should contain a valid JSON/Go
                        field access statement, such as desiredState.manifest.containers[2].
                        For example, if the object reference is to a container within
                        a pod, this would take on a value like: "spec.containers{name}"
                        (where "name" refers to the name of the container that triggered
                        the event) or if no container name is specified "spec.containers[2]"
                        (container with index 2 in this pod). This syntax is chosen
                        only to have some well-defined way of referencing a part of
                        an object. TODO: this design is not final and this field is
                        subject to change in the future.'
                      type: string
                    kind:
                      description: 'Kind of the referent. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#types-kinds'
                      type: string
                    name:
                      description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names'
                      type: string
                    namespace:
                      description: 'Namespace of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/'
                      type: string
                    resourceVersion:
                      description: 'Specific resourceVersion to which this reference
                        is made, if any. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#concurrency-control-and-consistency'
                      type: string
                    uid:
                      description: 'UID of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids'
                      type: string
                  type: object
                type: array
              lastScheduleMicroTime:
                description: Information about the last time the job was successfully
                  scheduled, with microsecond precision.
                format: date-time
                type: string
              lastScheduleTime:
                description: Information when was the last time the job was successfully
                  scheduled.
                format: date-time
                type: string
            type: object
        type: object
    served: true
    storage: true
    subresources:
      status: {}

or v1beta1 for v1.14

---
apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
  annotations:
    controller-gen.kubebuilder.io/version: (devel)
  creationTimestamp: null
  name: cronjobs.testdata.kubebuilder.io
spec:
  group: testdata.kubebuilder.io
  names:
    kind: CronJob
    listKind: CronJobList
    plural: cronjobs
    singular: mycronjob
  scope: Namespaced
  subresources:
    status: {}
  validation:
    openAPIV3Schema:
      description: CronJob is the Schema for the cronjobs API
      properties:
        apiVersion:
          description: 'APIVersion defines the versioned schema of this representation
            of an object. Servers should convert recognized schemas to the latest
            internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#resources'
          type: string
        kind:
          description: 'Kind is a string value representing the REST resource this
            object represents. Servers may infer this from the endpoint the client
            submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#types-kinds'
          type: string
        metadata:
          type: object
        spec:
          properties:
            schedule:
              type: string
          required:
          - schedule
          type: object
        status:
          description: CronJobStatus defines the observed state of CronJob
          properties:
            active:
              description: A list of pointers to currently running jobs.
              items:
                description: ObjectReference contains enough information to let you
                  inspect or modify the referred object.
                properties:
                  apiVersion:
                    description: API version of the referent.
                    type: string
                  fieldPath:
                    description: 'If referring to a piece of an object instead of
                      an entire object, this string should contain a valid JSON/Go
                      field access statement, such as desiredState.manifest.containers[2].
                      For example, if the object reference is to a container within
                      a pod, this would take on a value like: "spec.containers{name}"
                      (where "name" refers to the name of the container that triggered
                      the event) or if no container name is specified "spec.containers[2]"
                      (container with index 2 in this pod). This syntax is chosen
                      only to have some well-defined way of referencing a part of
                      an object. TODO: this design is not final and this field is
                      subject to change in the future.'
                    type: string
                  kind:
                    description: 'Kind of the referent. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#types-kinds'
                    type: string
                  name:
                    description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names'
                    type: string
                  namespace:
                    description: 'Namespace of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/'
                    type: string
                  resourceVersion:
                    description: 'Specific resourceVersion to which this reference
                      is made, if any. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#concurrency-control-and-consistency'
                    type: string
                  uid:
                    description: 'UID of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids'
                    type: string
                type: object
              type: array
            lastScheduleMicroTime:
              description: Information about the last time the job was successfully
                scheduled, with microsecond precision.
              format: date-time
              type: string
            lastScheduleTime:
              description: Information when was the last time the job was successfully
                scheduled.
              format: date-time
              type: string
          type: object
      type: object
  version: v1
  versions:
  - name: v1
    served: true
    storage: true

Testing this PR works against kind clusters

v1.17

$ k version                                         
Client Version: version.Info{Major:"1", Minor:"17", GitVersion:"v1.17.3", GitCommit:"06ad960bfd03b39c8310aaf92d1e7c12ce618213", GitTreeState:"clean", BuildDate:"2020-02-13T18:06:54Z", GoVersion:"go1.13.8", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"17", GitVersion:"v1.17.0", GitCommit:"70132b0f130acc0bed193d9ba59dd186f0e634cf", GitTreeState:"clean", BuildDate:"2020-01-14T00:09:19Z", GoVersion:"go1.13.4", Compiler:"gc", Platform:"linux/amd64"}

$ k apply -f ./testdata.kubebuilder.io_cronjobs.yaml
customresourcedefinition.apiextensions.k8s.io/cronjobs.testdata.kubebuilder.io created

v1.14.10

./kubectl version
Client Version: version.Info{Major:"1", Minor:"14", GitVersion:"v1.14.10", GitCommit:"575467a0eaf3ca1f20eb86215b3bde40a5ae617a", GitTreeState:"clean", BuildDate:"2019-12-11T12:41:00Z", GoVersion:"go1.12.12", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"14", GitVersion:"v1.14.10", GitCommit:"575467a0eaf3ca1f20eb86215b3bde40a5ae617a", GitTreeState:"clean", BuildDate:"2020-01-14T00:59:35Z", GoVersion:"go1.12.12", Compiler:"gc", Platform:"linux/amd64"}

./kubectl apply -f ./testdata.kubebuilder.io_cronjobs.yaml
customresourcedefinition.apiextensions.k8s.io/cronjobs.testdata.kubebuilder.io created

@DirectXMan12
Copy link
Contributor

/lgtm
/approve

/hold

Awesome. I suspect it was something around the actual Go types not having omitempty listed, which seems to have since been fixed. One last check -- can you double-check with v1beta1 as well? After that, feel free to cancel the hold.

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Apr 28, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: DirectXMan12, mcristina422, vincepri

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [DirectXMan12,vincepri]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

tcdowney added a commit to cloudfoundry/cf-k8s-networking that referenced this pull request Apr 28, 2020
- the presence of this section confused tooling such as `kapp`
  which would think our desired state of the CRD was to have
  an empty `status` conditions array
- long term this should be resolved by this change to controller-tools
  kubernetes-sigs/controller-tools#433 which we
  will adopt in https://www.pivotaltracker.com/story/show/172569440

[#172569406](https://www.pivotaltracker.com/story/show/172569406)
@mcristina422
Copy link
Contributor Author

Sorry I wasn't clear, I had to test v1beta1 above because v1 isnt included in k8s v1.14.10, it was added in v1.16

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 28, 2020
@k8s-ci-robot k8s-ci-robot merged commit 67819ee into kubernetes-sigs:master Apr 28, 2020
@akloss-cibo
Copy link

I've been trying to get this to work without much success. I'm not at all familiar with golang, so it's very possible I'm missing something obvious. Any ideas?

% git remote -v
origin	https://github.com/kubernetes-sigs/controller-tools.git (fetch)
origin	https://github.com/kubernetes-sigs/controller-tools.git (push)
% git pull
Already up to date.
% git rev-parse HEAD
b45abdba0dbb53e0c4dc90e1082d3802ecfb8be4
% git status
On branch master
Your branch is up to date with 'origin/master'.

nothing to commit, working tree clean
% go run -v -exec "./.run-in.sh pkg/crd/testdata" ./cmd/controller-gen crd:crdVersions=v1 paths=. output:dir=.
go: downloading golang.org/x/tools v0.0.0-20200616195046-dc31b401abb5
golang.org/x/tools/internal/event/label
golang.org/x/tools/internal/typesinternal
golang.org/x/tools/go/internal/gcimporter
golang.org/x/tools/internal/event/keys
golang.org/x/tools/internal/event/core
golang.org/x/tools/internal/event
golang.org/x/tools/internal/gocommand
golang.org/x/tools/internal/packagesinternal
golang.org/x/tools/go/internal/packagesdriver
golang.org/x/tools/go/gcexportdata
golang.org/x/tools/go/packages
sigs.k8s.io/controller-tools/pkg/loader
sigs.k8s.io/controller-tools/pkg/markers
sigs.k8s.io/controller-tools/pkg/genall/help
sigs.k8s.io/controller-tools/pkg/crd/markers
sigs.k8s.io/controller-tools/pkg/genall
sigs.k8s.io/controller-tools/pkg/rbac
sigs.k8s.io/controller-tools/pkg/webhook
sigs.k8s.io/controller-tools/pkg/deepcopy
sigs.k8s.io/controller-tools/pkg/crd
sigs.k8s.io/controller-tools/pkg/schemapatcher
sigs.k8s.io/controller-tools/cmd/controller-gen
% git diff
diff --git a/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml b/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml
index 72d44702..98665057 100644
--- a/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml
+++ b/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml
@@ -5202,3 +5202,9 @@ spec:
     storage: true
     subresources:
       status: {}
+status:
+  acceptedNames:
+    kind: ""
+    plural: ""
+  conditions: null
+  storedVersions: null
%

@alexellis
Copy link

@akloss-cibo same here, did you get any further? Asper #456 - I still see null instead of [].

@akloss-cibo
Copy link

Nope. I gave up and added

  ignoreDifferences:
    - group: apiextensions.k8s.io
      kind: CustomResourceDefinition
      jsonPointers:
        - /status

to my argocd application config instead. :(

damdo added a commit to damdo/cluster-autoscaler-operator that referenced this pull request Jun 6, 2022
removes empty status from generated crds due to: kubernetes-sigs/controller-tools#433
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants