-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Run scorecard tests in helm e2e tests #3768
Run scorecard tests in helm e2e tests #3768
Conversation
4/5 OLM scorecard tests fail, which should be expected. Also added some print statements to assist with debugging: Does not indicate which scorecard test failed: ``` • Failure [23.285 seconds] Integrating Helm Projects with OLM /home/austin/devel/operator-sdk/test/e2e-helm/e2e_helm_olm_test.go:32 with operator-sdk /home/austin/devel/operator-sdk/test/e2e-helm/e2e_helm_olm_test.go:33 should generate and run a valid OLM bundle and packagemanifests [It] /home/austin/devel/operator-sdk/test/e2e-helm/e2e_helm_olm_test.go:50 Expected <v1alpha3.State>: pass to equal <v1alpha3.State>: fail ``` Additional Output clearly shows which olm scorecard tests did not match expectations: ``` - Name: olm-crds-have-validation Expected: fail Output: fail - Name: olm-spec-descriptors Expected: fail Output: fail - Name: olm-bundle-validation Expected: fail Output: pass ```
expected := make(map[string]v1alpha3.State) | ||
expected[OLMBundleValidationTest] = v1alpha3.PassState | ||
expected[OLMCRDsHaveResourcesTest] = v1alpha3.FailState | ||
expected[OLMCRDsHaveValidationTest] = v1alpha3.FailState |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the OLMCRDsHaveValidationTest
should fail? Why this test is not returning a passing state? IMO we have a bug here in the scorecard for Helm/Ansible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was noticed before we cut 1.0. I wasn't in the meeting that discussed it, but if we do want to make a change here, it is in the scaffolding, so out of scope for this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference using helm operator-sdk-sample:
$ cd ~/devel/operator-sdk-samples/helm/memcached-operator
$ operator-sdk scorecard ./bundle --selector=suite=olm
SNIP...
--------------------------------------------------------------------------------
Image: quay.io/operator-framework/scorecard-test:master
Entrypoint: [scorecard-test olm-crds-have-validation]
Labels:
"suite":"olm"
"test":"olm-crds-have-validation-test"
Results:
Name: olm-crds-have-validation
State: fail
Suggestions:
Add CRD validation for spec field `metrics` in Memcached/v1alpha1
Add CRD validation for spec field `podAnnotations` in Memcached/v1alpha1
Add CRD validation for spec field `updateStrategy` in Memcached/v1alpha1
Add CRD validation for spec field `extraContainers` in Memcached/v1alpha1
Add CRD validation for spec field `image` in Memcached/v1alpha1
Add CRD validation for spec field `kind` in Memcached/v1alpha1
Add CRD validation for spec field `pdbMinAvailable` in Memcached/v1alpha1
Add CRD validation for spec field `replicaCount` in Memcached/v1alpha1
Add CRD validation for spec field `securityContext` in Memcached/v1alpha1
Add CRD validation for spec field `tolerations` in Memcached/v1alpha1
Add CRD validation for spec field `affinity` in Memcached/v1alpha1
Add CRD validation for spec field `memcached` in Memcached/v1alpha1
Add CRD validation for spec field `resources` in Memcached/v1alpha1
Add CRD validation for spec field `serviceAnnotations` in Memcached/v1alpha1
Add CRD validation for spec field `AntiAffinity` in Memcached/v1alpha1
Add CRD validation for spec field `nodeSelector` in Memcached/v1alpha1
Add CRD validation for spec field `extraVolumes` in Memcached/v1alpha1
Log:
Loaded 1 Custom Resources from alm-examples
Loaded CustomresourceDefinitions: [&CustomResourceDefinition{ObjectMeta:{memcacheds.cache.example.com 0 0001-01-01 00:00:00 +0000 UTC <nil> <nil> map[] map[] [] [] []},Spec:CustomResourceDefinitionSpec{Group:cache.example.com,Names:CustomResourceDefinitionNames{Plural:memcacheds,Singular:memcached,ShortNames:[],Kind:Memcached,ListKind:MemcachedList,Categories:[],},Scope:Namespaced,Versions:[]CustomResourceDefinitionVersion{CustomResourceDefinitionVersion{Name:v1alpha1,Served:true,Storage:true,Schema:&CustomResourceValidation{OpenAPIV3Schema:&JSONSchemaProps{ID:,Schema:,Ref:nil,Description:Memcached is the Schema for the memcacheds API,Type:object,Format:,Title:,Default:nil,Maximum:nil,ExclusiveMaximum:false,Minimum:nil,ExclusiveMinimum:false,MaxLength:nil,MinLength:nil,Pattern:,MaxItems:nil,MinItems:nil,UniqueItems:false,MultipleOf:nil,Enum:[]JSON{},MaxProperties:nil,MinProperties:nil,Required:[],Items:nil,AllOf:[]JSONSchemaProps{},OneOf:[]JSONSchemaProps{},AnyOf:[]JSONSchemaProps{},Not:nil,Properties:map[string]JSONSchemaProps{apiVersion: { <nil> 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/sig-architecture/api-conventions.md#resources string nil <nil> false <nil> false <nil> <nil> <nil> <nil> false <nil> [] <nil> <nil> [] nil [] [] [] nil map[] nil map[] map[] nil map[] nil nil false <nil> false false [] <nil> <nil>},kind: { <nil> 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/sig-architecture/api-conventions.md#types-kinds string nil <nil> false <nil> false <nil> <nil> <nil> <nil> false <nil> [] <nil> <nil> [] nil [] [] [] nil map[] nil map[] map[] nil map[] nil nil false <nil> false false [] <nil> <nil>},metadata: { <nil> object nil <nil> false <nil> false <nil> <nil> <nil> <nil> false <nil> [] <nil> <nil> [] nil [] [] [] nil map[] nil map[] map[] nil map[] nil nil false <nil> false false [] <nil> <nil>},spec: { <nil> Spec defines the desired state of Memcached object nil <nil> false <nil> false <nil> <nil> <nil> <nil> false <nil> [] <nil> <nil> [] nil [] [] [] nil map[] nil map[] map[] nil map[] nil nil false 0xc0004c8778 false false [] <nil> <nil>},status: { <nil> Status defines the observed state of Memcached object nil <nil> false <nil> false <nil> <nil> <nil> <nil> false <nil> [] <nil> <nil> [] nil [] [] [] nil map[] nil map[] map[] nil map[] nil nil false 0xc0004c8779 false false [] <nil> <nil>},},AdditionalProperties:nil,PatternProperties:map[string]JSONSchemaProps{},Dependencies:JSONSchemaDependencies{},AdditionalItems:nil,Definitions:JSONSchemaDefinitions{},ExternalDocs:nil,Example:nil,Nullable:false,XPreserveUnknownFields:nil,XEmbeddedResource:false,XIntOrString:false,XListMapKeys:[],XListType:nil,XMapType:nil,},},Subresources:&CustomResourceSubresources{Status:&CustomResourceSubresourceStatus{},Scale:nil,},AdditionalPrinterColumns:[]CustomResourceColumnDefinition{},},},Conversion:nil,PreserveUnknownFields:false,},Status:CustomResourceDefinitionStatus{Conditions:[]CustomResourceDefinitionCondition{},AcceptedNames:CustomResourceDefinitionNames{Plural:,Singular:,ShortNames:[],Kind:,ListKind:,Categories:[],},StoredVersions:[],},}]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @asmacdo,
It is true: since in tHelm/Ansible we have not the go types definition and we do not all the make manifests (controller-gen) the CRD has not the CR specs and their validations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I raised : #3770 let's see what others think as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Great work 👍
4/5 OLM scorecard tests fail, which should be expected.
Also added some print statements to assist with debugging:
Does not indicate which scorecard test failed:
Additional Output clearly shows which olm scorecard tests did not match
expectations:
Description of the change:
Motivation for the change:
Checklist
If the pull request includes user-facing changes, extra documentation is required:
changelog/fragments
(seechangelog/fragments/00-template.yaml
)website/content/en/docs