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

Improve the check for KUDO CRDs when creating a client #1037

Merged
merged 2 commits into from
Nov 8, 2019

Conversation

nfnt
Copy link
Member

@nfnt nfnt commented Nov 7, 2019

What this PR does / why we need it:
CRDs aren't tied to a namespace. By checking for them directly through the 'apiextensions' client is a cleaner approach than checking if listing CRDs objects succeed. This also make the namespace parameter of kudo.NewClient redundant.

CRDs aren't tied to a namespace. By checking for them directly through the 'apiextensions' client is a cleaner approach than checking if listing CRDs objects succeed.
Copy link
Contributor

@alenkacz alenkacz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nfnt
Copy link
Member Author

nfnt commented Nov 7, 2019

Going to check if this CI failure is a flake or actually caused by the change:

Failed
logger.go:37: 14:55:11 | create-or-update | Creating namespace: kudo-test-careful-wolf
logger.go:37: 14:55:11 | create-or-update/0-crd | starting test step 0-crd
logger.go:37: 14:55:11 | create-or-update/0-crd | CustomResourceDefinition:/prowjobs.prow.k8s.io created
logger.go:37: 14:55:41 | create-or-update/0-crd | test step failed 0-crd
case.go:154: --- CustomResourceDefinition:/prowjobs.prow.k8s.io
    +++ CustomResourceDefinition:/prowjobs.prow.k8s.io
    @@ -2,18 +2,123 @@
     kind: CustomResourceDefinition
     metadata:
       name: prowjobs.prow.k8s.io
    +spec:
    +  additionalPrinterColumns:
    +  - JSONPath: .spec.job
    +    description: The name of the job being run.
    +    name: Job
    +    type: string
    +  - JSONPath: .status.build_id
    +    description: The ID of the job being run.
    +    name: BuildId
    +    type: string
    +  - JSONPath: .spec.type
    +    description: The type of job being run.
    +    name: Type
    +    type: string
    +  - JSONPath: .spec.refs.org
    +    description: The org for which the job is running.
    +    name: Org
    +    type: string
    +  - JSONPath: .spec.refs.repo
    +    description: The repo for which the job is running.
    +    name: Repo
    +    type: string
    +  - JSONPath: .spec.refs.pulls[*].number
    +    description: The pulls for which the job is running.
    +    name: Pulls
    +    type: string
    +  - JSONPath: .status.startTime
    +    description: When the job started running.
    +    name: StartTime
    +    type: date
    +  - JSONPath: .status.completionTime
    +    description: When the job finished running.
    +    name: CompletionTime
    +    type: date
    +  - JSONPath: .status.state
    +    description: The state of the job.
    +    name: State
    +    type: string
    +  conversion:
    +    strategy: None
    +  group: prow.k8s.io
    +  names:
    +    kind: ProwJob
    +    listKind: ProwJobList
    +    plural: prowjobs
    +    singular: prowjob
    +  preserveUnknownFields: true
    +  scope: Namespaced
    +  validation:
    +    openAPIV3Schema:
    +      properties:
    +        spec:
    +          properties:
    +            max_concurrency:
    +              minimum: 0
    +              type: integer
    +            type:
    +              enum:
    +              - presubmit
    +              - postsubmit
    +              - periodic
    +              - batch
    +              type: string
    +        status:
    +          anyOf:
    +          - not:
    +              properties:
    +                state:
    +                  enum:
    +                  - success
    +                  - failure
    +                  - error
    +                  - aborted
    +                  type: string
    +          - required:
    +            - completionTime
    +          properties:
    +            state:
    +              enum:
    +              - triggered
    +              - pending
    +              - success
    +              - failure
    +              - aborted
    +              - error
    +              type: string
    +  version: v1
    +  versions:
    +  - name: v1
    +    served: true
    +    storage: true
     status:
       acceptedNames:
         kind: ProwJob
    +    listKind: ProwJobList
    +    plural: prowjobs
    +    singular: prowjob
       conditions:
    -  - message: no conflicts found
    +  - lastTransitionTime: "2019-11-07T14:55:11Z"
    +    message: no conflicts found
         reason: NoConflicts
         status: "True"
         type: NamesAccepted
    -  - message: the initial names have been accepted
    +  - lastTransitionTime: null
    +    message: the initial names have been accepted
         reason: InitialNamesAccepted
         status: "True"
         type: Established
    +  - lastTransitionTime: "2019-11-07T14:55:11Z"
    +    message: '[spec.validation.openAPIV3Schema.properties[spec].type: Required value:
    +      must not be empty for specified object fields, spec.validation.openAPIV3Schema.properties[status].anyOf[0].not.properties[state].type:
    +      Forbidden: must be empty to be structural, spec.validation.openAPIV3Schema.properties[status].type:
    +      Required value: must not be empty for specified object fields, spec.validation.openAPIV3Schema.type:
    +      Required value: must not be empty at the root]'
    +    reason: Violations
    +    status: "True"
    +    type: NonStructuralSchema
       storedVersions:
       - v1
     
    
case.go:154: resource CustomResourceDefinition:/prowjobs.prow.k8s.io: .status.conditions: slice length mismatch: 2 != 3
case.go:157: failed in step 0-crd
logger.go:37: 14:55:41 | create-or-update | create-or-update events from ns kudo-test-careful-wolf:
logger.go:37: 14:55:41 | create-or-update | Deleting namespace: kudo-test-careful-wolf

@nfnt
Copy link
Member Author

nfnt commented Nov 7, 2019

The issue mentioned above will be fixed in #1039 and is unrelated to this diff.

@nfnt nfnt merged commit 2a585ae into master Nov 8, 2019
@nfnt nfnt deleted the nfnt/check-for-crds branch November 8, 2019 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants