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

operators postgresql (4.7.5) #955

Merged
merged 1 commit into from
Apr 22, 2022
Merged

Conversation

ValClarkson
Copy link
Contributor

Thanks submitting your Operator. Please check below list before you create your Pull Request.

New Submissions

Updates to existing Operators

  • Did you create a ci.yaml file according to the update instructions?
  • Is your new CSV pointing to the previous version with the replaces property if you chose replaces-mode via the updateGraph property in ci.yaml?
  • Is your new CSV referenced in the appropriate channel defined in the package.yaml or annotations.yaml ?
  • Have you tested an update to your Operator when deployed via OLM?
  • Is your submission signed?

Your submission should not

  • Modify more than one operator
  • Modify an Operator you don't own
  • Rename an operator - please remove and add with a different name instead
  • Modify any files outside the above mentioned folders
  • Contain more than one commit. Please squash your commits.

Operator Description must contain (in order)

  1. Description about the managed Application and where to find more information
  2. Features and capabilities of your Operator and how to use it
  3. Any manual steps about potential pre-requisites for using your Operator

Operator Metadata should contain

  • Human readable name and 1-liner description about your Operator
  • Valid category name1
  • One of the pre-defined capability levels2
  • Links to the maintainer, source code and documentation
  • Example templates for all Custom Resource Definitions intended to be used
  • A quadratic logo

Remember that you can preview your CSV here.

--

1 If you feel your Operator does not fit any of the pre-defined categories, file an issue against this repo and explain your need

2 For more information see here

@openshift-ci openshift-ci bot requested review from J0zi and mvalarh March 23, 2022 22:28
@github-actions
Copy link
Contributor

Dockerfile or bundle.Dockerfile is added/changed. Note that for security reasons none of these files are going to be used when building bundle. Docker file will be generated and all label information is taken from annotations.yaml.

@github-actions github-actions bot changed the title Add PGO v4.7.5 operators postgresql (4.7.5) Mar 23, 2022
@github-actions
Copy link
Contributor

Dear @ValClarkson,
There are some operators version that are using deprecated api and kubernetes max versions (operatorhub.io/ui-metadata-max-k8s-version) is NOT set correctly under annotation field in CSV file.

Affected versions : postgresql:v4.7.5

More info in 'Kubernetes max version in CSV' section here.

@github-actions
Copy link
Contributor

Dockerfile or bundle.Dockerfile is added/changed. Note that for security reasons none of these files are going to be used when building bundle. Docker file will be generated and all label information is taken from annotations.yaml.

@github-actions
Copy link
Contributor

Dockerfile or bundle.Dockerfile is added/changed. Note that for security reasons none of these files are going to be used when building bundle. Docker file will be generated and all label information is taken from annotations.yaml.

@github-actions
Copy link
Contributor

Dockerfile or bundle.Dockerfile is added/changed. Note that for security reasons none of these files are going to be used when building bundle. Docker file will be generated and all label information is taken from annotations.yaml.

@github-actions
Copy link
Contributor

Dockerfile or bundle.Dockerfile is added/changed. Note that for security reasons none of these files are going to be used when building bundle. Docker file will be generated and all label information is taken from annotations.yaml.

@J0zi
Copy link
Contributor

J0zi commented Mar 30, 2022

@github-actions
Copy link
Contributor

Dockerfile or bundle.Dockerfile is added/changed. Note that for security reasons none of these files are going to be used when building bundle. Docker file will be generated and all label information is taken from annotations.yaml.

Signed-off-by: ValClarkson <[email protected]>
@github-actions
Copy link
Contributor

Dockerfile or bundle.Dockerfile is added/changed. Note that for security reasons none of these files are going to be used when building bundle. Docker file will be generated and all label information is taken from annotations.yaml.

@J0zi
Copy link
Contributor

J0zi commented Apr 21, 2022

@ValClarkson please fix validation errors. Invalid CRD schema, unable to load/parse CRD into an object.

@ValClarkson
Copy link
Contributor Author

@ValClarkson please fix validation errors. Invalid CRD schema, unable to load/parse CRD into an object.

HI, @J0zi Those errors are related to operator-sdk using the 1.23 k8s library blocking the bundle validation. However, this version of our operator is targeted for k8s 1.21 and below. I think this is being incorrectly flagged, that being said is there any way to get this operator validated with a previous version of the operator-sdk for validation?

@mvalarh
Copy link
Contributor

mvalarh commented Apr 22, 2022

@ValClarkson You can ignore warnings, but errors should be fixed. One of them is Field spec.preserveUnknownFields, Value 0xc0008c3e29: spec.preserveUnknownFields: Invalid value: true: cannot set to true, set x-preserve-unknown-fields to true in spec.versions[*].schema instead

@J0zi J0zi merged commit 6253f0f into k8s-operatorhub:main Apr 22, 2022
@camilamacedo86
Copy link
Contributor

camilamacedo86 commented Apr 22, 2022

Hi @ValClarkson,

HI, @J0zi Those errors are related to operator-sdk using the 1.23 k8s library blocking the bundle validation. However, this version of our operator is targeted for k8s 1.21 and below. I think this is being incorrectly flagged, that being said is there any way to get this operator validated with a previous version of the operator-sdk for validation?

That does not seem accurate. if the problem was trying to parse the CRD v1beta1 with k8s api 1.23 this error would not be raised at all. v1beta1 version does NOT exist from k8s api 1.22 and the whole api is not found.

See: https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#field-pruning and check this doc: https://kubernetes.io/blog/2019/06/20/crd-structural-schema/#pruning-don-t-preserve-unknown-fields ( from k8s 1.15 )

@camilamacedo86
Copy link
Contributor

camilamacedo86 commented Apr 22, 2022

Hi @J0zi and @ValClarkson,

See that operator-sdk bundle validate ( using SDK latest version ) will not raise this error with v1beta1 CRDs:

  1. Create a mock project using v1beta1 CRDs
mkdir test
cd test
operator-sdk init
operator-sdk create api --group crew --version v1 --kind Admiral --controller=true --resource=true --namespaced=false --make=false --crd-version=v1beta1
go mod tidy
make bundle
  1. Using bundle validate check:
$ operator-sdk bundle validate ./bundle
INFO[0000] All validation tests have completed successfully 

OR

$ operator-sdk bundle validate ./bundle --select-optional suite=operatorframework
WARN[0000] Warning: Value : (test.v0.0.1) csv.Spec.minKubeVersion is not informed. It is recommended you provide this information. Otherwise, it would mean that your operator project can be distributed and installed in any cluster version available, which is not necessarily the case for all projects. 
WARN[0000] Warning: Value test.v0.0.1: this bundle is using APIs which were deprecated and removed in v1.22. More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. Migrate the API(s) for CRD: (["admirals.crew.my.domain"]) 
ERRO[0000] Error: Value : (test.v0.0.1) csv.Spec.Icon elements should contain both data and mediatype 
WARN[0000] Warning: Value test.v0.0.1: this bundle is using APIs which were deprecated and removed in v1.22. More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. Migrate the API(s) for CRD: (["admirals.crew.my.domain"]) 

Then, as you can see the statement is inaccurate.

@camilamacedo86
Copy link
Contributor

@cbandy
Copy link

cbandy commented Apr 22, 2022

@camilamacedo86 The issue is with preserveUnknownFields=true, not just v1beta1.

Your example for v1beta1 is accurate, but it doesn't set preserveUnknownFields=true. This version (4.7.5) of our operator only functions properly when this setting is true (or the default of true.)

When I add that detail to your steps, I see different results depending on operator-sdk version. Validation passes for me locally using v1.16.0 from January of this year.

$ operator-sdk version
operator-sdk version: "v1.16.0", commit: "560044140c4f3d88677e4ef2872931f5bb97f255", kubernetes version: "1.21", go version: "go1.16.13", GOOS: "darwin", GOARCH: "amd64"

$ mkdir test
$ cd test
$ go mod init test
go: creating new go.mod: module test

$ operator-sdk init
$ operator-sdk create api --group crew --version v1 --kind Admiral --controller=true --resource=true --namespaced=false --make=false --crd-version=v1beta1
$ go mod tidy

$ make bundle

operator-sdk bundle validate ./bundle
INFO[0000] All validation tests have completed successfully

# Above is the same as your original steps so far. The following is the different.

$ sed -i '' -e '/CRD_OPTIONS/ s/Fields=false/Fields=true/' Makefile
$ grep CRD_OPTIONS Makefile
CRD_OPTIONS ?= "crd:crdVersions={v1beta1},trivialVersions=true,preserveUnknownFields=true"
        $(CONTROLLER_GEN) $(CRD_OPTIONS) rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases

$ make bundle

operator-sdk bundle validate ./bundle
INFO[0000] All validation tests have completed successfully

The validation fails when I repeat this steps with a newer SDK.

$ operator-sdk version
operator-sdk version: "v1.19.1", commit: "079d8852ce5b42aa5306a1e33f7ca725ec48d0e3", kubernetes version: "1.23", go version: "go1.17.8", GOOS: "darwin", GOARCH: "amd64"



# Above is the same as your original steps so far. The following is the different.

$ sed -i '' -e '/CRD_OPTIONS/ s/Fields=false/Fields=true/' Makefile
$ grep CRD_OPTIONS Makefile
CRD_OPTIONS ?= "crd:crdVersions={v1beta1},trivialVersions=true,preserveUnknownFields=true"
        $(CONTROLLER_GEN) $(CRD_OPTIONS) rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases

$ make bundle

ERRO[0000] Error: Field spec.preserveUnknownFields, Value 0xc0008ab590: spec.preserveUnknownFields: Invalid value: true: cannot set to true, set x-preserve-unknown-fields to true in spec.versions[*].schema instead
make: *** [bundle] Error 1

@camilamacedo86
Copy link
Contributor

@cbandy,

Thank you for sharing there. If we compare the CRD generate in both scenarios are they == or can we find any diff?
We will need to check it further to understand better. Could you please raise an issue in the SDK with the content #955 (comment) repo and ping me there?

@acornett21
Copy link
Collaborator

Hi @J0zi and @ValClarkson,

See the PR with it fixed : https://github.com/k8s-operatorhub/community-operators/pull/1108/files

@camilamacedo86 Setting the values to false does not actually work, and results in the same error from operator-sdk (well the k8's 1.23 library). The only way this works is on older versions of operator-sdk that does not have the 1.23 k8's upstream port.

@camilamacedo86
Copy link
Contributor

camilamacedo86 commented Apr 27, 2022

Hi @acornett21,

Hi @J0zi and @ValClarkson,

See the PR with it fixed : https://github.com/k8s-operatorhub/community-operators/pull/1108/files

@camilamacedo86 Setting the values to false does not actually work, and results in the same error from operator-sdk (well the k8's 1.23 library). The only way this works is on older versions of operator-sdk that does not have the 1.23 k8's upstream port.

It is not true. By applying the changes https://github.com/k8s-operatorhub/community-operators/pull/1108/files the error is not raised. I tested it locally. If you are guessing the root cause here with operator-sdk (well the k8's 1.23 library) by looking at the k8s deps on SDK CLI go mod be aware that they are not used on these checks at all.

SDK only call the checks that are implemented on the operator framework/api. Indeed, SDK is not responsible for the checks but only to call them.

@camilamacedo86
Copy link
Contributor

Hi @cbandy,

I did the steps described (#955 (comment)) with SDK master branch:

$ operator-sdk version
operator-sdk version: "v1.15.0-97-g04fb8449", commit: "04fb844950722eef4e70fe7a1bfc656c712a7a85", kubernetes version: "v1.23", go version: "go1.17.5", GOOS: "darwin", GOARCH: "amd64"
camilamacedo86@cmacedo-mac:~/go/src/test$ make bundle
/Users/camilamacedo86/go/src/test/bin/controller-gen "crd:crdVersions={v1beta1},trivialVersions=true,preserveUnknownFields=false" rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
operator-sdk generate kustomize manifests -q
cd config/manager && /Users/camilamacedo86/go/src/test/bin/kustomize edit set image controller=controller:latest
/Users/camilamacedo86/go/src/test/bin/kustomize build config/manifests | operator-sdk generate bundle -q --overwrite --version 0.0.1  
INFO[0000] Creating bundle.Dockerfile                   
INFO[0000] Creating bundle/metadata/annotations.yaml    
INFO[0000] Bundle metadata generated suceessfully       
operator-sdk bundle validate ./bundle
INFO[0000] All validation tests have completed successfully

See:

$ operator-sdk bundle validate ./bundle --select-optional suite=operatorframework 
WARN[0000] Warning: Value : (test.v0.0.1) csv.Spec.minKubeVersion is not informed. It is recommended you provide this information. Otherwise, it would mean that your operator project can be distributed and installed in any cluster version available, which is not necessarily the case for all projects. 
WARN[0000] Warning: Value test.v0.0.1: this bundle is using APIs which were deprecated and removed in v1.22. More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. Migrate the API(s) for CRD: (["admirals.crew.my.domain"]) 
ERRO[0000] Error: Value : (test.v0.0.1) csv.Spec.Icon elements should contain both data and mediatype 
WARN[0000] Warning: Value test.v0.0.1: this bundle is using APIs which were deprecated and removed in v1.22. More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. Migrate the API(s) for CRD: (["admirals.crew.my.domain"]) 

And I still unable to reproduce the scenario because controller-gen will be setting in this case the preserveUnknownFields: false by default.

See:

$ cat manifests/crew.my.domain_admirals.yaml 
apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
  annotations:
    controller-gen.kubebuilder.io/version: v0.6.2
  creationTimestamp: null
  name: admirals.crew.my.domain
spec:
  group: crew.my.domain
  names:
    kind: Admiral
    listKind: AdmiralList
    plural: admirals
    singular: admiral
  preserveUnknownFields: false
  scope: Cluster

The error can only be found if we remove the preserveUnknownFields: false and run the check again:

$ operator-sdk bundle validate ./bundle --select-optional suite=operatorframework 
ERRO[0000] Error: Field spec.preserveUnknownFields, Value 0xc000c31ac0: spec.preserveUnknownFields: Invalid value: true: cannot set to true, set x-preserve-unknown-fields to true in spec.versions[*].schema instead 

Then, by checking the above bundle ( without the spec.preserveUnknownFields ) with SDK "v1.16.0" the error is not faced but it begins to be faced when I use SDK 1.17.0.

See that SDK 1.17.0 does not use k8s 1.23:

$ operator-sdk version
operator-sdk version: "v1.17.0", commit: "704b02a9ba86e85f43edb1b20457859e9eedc6e6", kubernetes version: "v1.21", go version: "go1.17.5", GOOS: "darwin", GOARCH: "amd64"

So the statement about the root cause here is not accurate.
However, we need to check why the error were not faced before the SDK release 1.17.0.

@camilamacedo86
Copy link
Contributor

The issue operator-framework/operator-sdk#5688 was created for we are able to analyse this scenario.

@acornett21
Copy link
Collaborator

@camilamacedo86 I think the output of operator-sdk version for 1.17.0 might be incorrect, since if I look at the release notes k8 1.23 was merged into version 1.17.0 which is when this issue presented itself. See release notes here and this PR.

@acornett21
Copy link
Collaborator

To add more clarity, or point out where this error comes from it is coming from here in the k8's library.

@camilamacedo86
Copy link
Contributor

Hi @acornett21 that is true. Really thank you for share that.

@acornett21
Copy link
Collaborator

@camilamacedo86 No problem, thanks for creating an issue in operator-sdk for this. Let me know if you have any more questions around this use case.

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

Successfully merging this pull request may close these issues.

6 participants