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

Update operator-sdk version #74

Merged
merged 13 commits into from
Feb 25, 2020

Conversation

andrewazores
Copy link
Member

@andrewazores andrewazores commented Feb 19, 2020

This updates the operator-sdk version and related generated files. In my testing this works up to the point of make image and make deploy, and the resulting operator logs show that the sdk version is indeed 0.15.2 and everything appears to work correctly. Not yet done is replacing operator-courier and using the newer sdk version to create the operator bundles.

@andrewazores andrewazores force-pushed the update-operator-sdk branch 2 times, most recently from 9bb3814 to b2d5107 Compare February 19, 2020 16:44
@andrewazores andrewazores changed the title Update operator-sdk version [WIP] Update operator-sdk version Feb 19, 2020
@andrewazores
Copy link
Member Author

@ebaron I had to do a lot of messing about with go module stuff before getting this to work, so please test out make image and make deploy and see if that much works for you too before I go any further on with the bundling work.

Makefile Outdated Show resolved Hide resolved
@andrewazores andrewazores force-pushed the update-operator-sdk branch 2 times, most recently from 9ad86b2 to 1dc1add Compare February 20, 2020 00:22
@andrewazores
Copy link
Member Author

I'm still working on the bundling part. It seems like the bundles created by operator-sdk are not the same thing as what operator-courier made. Quay accepts uploads of them, but I haven't yet tested what happens if you try to consume that from the marketplace. operator-sdk bundle validate doesn't like them itself, which is weird.

@andrewazores andrewazores force-pushed the update-operator-sdk branch 2 times, most recently from bed3bf5 to 850f7dd Compare February 20, 2020 13:47
@andrewazores
Copy link
Member Author

I think these docs are relevant for this bundling format, but I'm still trying to piece it together. I haven't found anything like a migration guide or explanation of this bundling vs operator-courier. Maybe I should just push it to Quay and see what my cluster's marketplace makes of it.

https://github.com/operator-framework/operator-sdk/blob/master/doc/proposals/sdk-integration-with-olm.md

https://github.com/operator-framework/operator-registry/#manifest-format

https://github.com/openshift/enhancements/blob/master/enhancements/olm/operator-bundle.md#docker

@andrewazores
Copy link
Member Author

I'm not sure how to see any logs the marketplace/OperatorHub might be producing, but it looks like it's silently ignoring my custom operator source when I point it at my namespace and the latest bundle is one produced by operator-sdk. The bundle produced does look very different from what operator-courier does when inspected manually, so I really don't know what to do with this bundle format.

@andrewazores
Copy link
Member Author

Ah, okay. Part of my issue was with the podman save in the bundle-upload script I made, which was spitting out a different filesystem structure than what went into it. However, I'm still not sure what use the sdk-created bundle image is. It ends up looking like a versioned subdirectory similar to what operator-courier would produce, packaged into a container image, but quay.io won't allow that to be uploaded unless you add the force parameter, and then it still doesn't work with OperatorHub.

@andrewazores
Copy link
Member Author

Here's a comparison from my namespace of 0.3.3 (made with operator-courier before this PR) and 0.4.0 (made with operator-sdk after this PR and some local changes):

$ tree
.
├── andrewazores_container-jfr-operator-bundle_0.3.3
│   └── container-jfr-operator-bundle-54tn2q3f
│       ├── 0.3.1
│       │   ├── container-jfr-operator-bundle.v0.3.1.clusterserviceversion.yaml
│       │   ├── containerjfrs.rhjmc.redhat.com.crd.yaml
│       │   └── flightrecorders.rhjmc.redhat.com.crd.yaml
│       ├── 0.3.2
│       │   ├── container-jfr-operator-bundle.v0.3.1.clusterserviceversion.yaml
│       │   ├── containerjfrs.rhjmc.redhat.com.crd.yaml
│       │   └── flightrecorders.rhjmc.redhat.com.crd.yaml
│       ├── 0.3.3
│       │   ├── container-jfr-operator-bundle.v0.3.3.clusterserviceversion.yaml
│       │   ├── containerjfrs.rhjmc.redhat.com.crd.yaml
│       │   └── flightrecorders.rhjmc.redhat.com.crd.yaml
│       ├── container-jfr-operator-bundle.package.yaml
│       └── openshift
│           └── operator-source.yaml
└── andrewazores_container-jfr-operator-bundle_0.4.0
    ├── container-jfr-operator-bundle.v0.4.0.clusterserviceversion.yaml
    ├── metadata
    │   └── annotations.yaml
    ├── rhjmc.redhat.com_containerjfrs_crd.yaml
    └── rhjmc.redhat.com_flightrecorders_crd.yaml

@andrewazores andrewazores force-pushed the update-operator-sdk branch 4 times, most recently from 9198324 to 18535cc Compare February 20, 2020 21:58
@ebaron
Copy link
Member

ebaron commented Feb 20, 2020

It's a bit confusing, but my impression is that this new operator-registry format doesn't use an application repository like we currently do. I think we need to add the steps in
https://github.com/operator-framework/operator-registry/#building-an-index-of-operators-using-opm
to our Makefile.

  1. Push the bundle to Quay as normal container image.
  2. Build an index using opm, which you can get here: https://github.com/operator-framework/operator-registry/releases/download/v1.5.9/linux-amd64-opm
  3. Push that index image to Quay as well, as a container image.

Going by https://github.com/operator-framework/operator-registry/#using-the-index-with-operator-lifecycle-manager, I think we then need to replace our OperatorSource with a CatalogSource pointing to our index image. Then a user can access the operator, by applying the CatalogSource to their cluster.

I could very well be wrong, but that's my interpretation of how this all works now.

@andrewazores
Copy link
Member Author

That was a great tip, I think I've made some more progress. Testing it out currently. But, in the meantime before I forget - if you try that out yourself, you'll currently need to build operator-framework/operator-registry manually and get a local copy of opm, because the released version has a bug where it incorrectly copies itself into the builder image, so the index creation fails. The fix is committed but not released.

@andrewazores
Copy link
Member Author

That last commit /seems/ to work using that new bundling/index/catalog system. The catalog-source points to an index in my personal Quay namespace for the time being, and all the bundles indexed are also in my own namespace. Those will be cleaned up once we've verified this all works correctly and we can generate those bundles and the index and push them to the team namespace.

@andrewazores andrewazores force-pushed the update-operator-sdk branch 4 times, most recently from cdb38d9 to 757b98b Compare February 21, 2020 19:53
@ebaron
Copy link
Member

ebaron commented Feb 21, 2020

I ran through the steps myself, and everything seems to work. Nice work!

@andrewazores andrewazores force-pushed the update-operator-sdk branch 3 times, most recently from 25508e4 to 6378fee Compare February 24, 2020 13:37
@andrewazores andrewazores force-pushed the update-operator-sdk branch 2 times, most recently from 775a4f6 to ddb2228 Compare February 24, 2020 13:52
@andrewazores
Copy link
Member Author

Sorry for the dozen force-pushes that just happened. I went through and cleaned up the two WIP commits, but also realized there was a left behind "clean-bundle" Make recipe, and then tried scorecard testing and ran into a few CRD issues. Also realized that I had copied in several newer CSV versions that were from my personal namespace and which don't reflect versioning in the real upstream repositories.

I think this is more or less cleaned up and ready now. Scorecard testing still fails with the following however:

INFO[0000] Using config file: /home/andrew/workspace/container-jfr-operator/.osdk-scorecard.yaml 
ERRO[0179] Plugin `OLM Integration` failed with error (error validating ClusterServiceVersion: Error: Value apiextensions.k8s.io/v1alpha1, Kind=FlightRecorder: example must have a provided API
Error: Value apiextensions.k8s.io/v1alpha1, Kind=ContainerJFR: example must have a provided API
) 
basic:
	Writing into CRs has an effect      : pass
	Labels: 
		"suite":"basic"
		"test":"writingintocrshaseffecttest"
		"necessity":"required"

	Spec Block Exists                   : pass
	Labels: 
		"necessity":"required"
		"suite":"basic"
		"test":"checkspectest"

	Status Block Exists                 : fail
	Labels: 
		"suite":"basic"
		"test":"checkstatustest"
		"necessity":"required"
	Suggestions: 
		Add a 'status' field to your Custom Resource
		Add a 'status' field to your Custom Resource


make: *** [Makefile:79: scorecard] Error 1

@ebaron
Copy link
Member

ebaron commented Feb 24, 2020

That's odd, I wonder if they expect us to remove omitempty from the Spec and Status fields now.

@andrewazores
Copy link
Member Author

Not sure. I can take a further look into it separately though if that's okay.

@andrewazores
Copy link
Member Author

I'm also not sure what it's complaining about with the example must have a provided API failure. It's an assertion against the CSV, so the metadata.annotations.alm-examples maybe? But what we have looks like it matches the examples given.

@ebaron
Copy link
Member

ebaron commented Feb 24, 2020

It looks like the group name is wrong for the alm-examples. Should be rhjmc.redhat.com instead of apiextensions.k8s.io.

@andrewazores
Copy link
Member Author

andrewazores commented Feb 24, 2020

Yep, the group name part fixed the examples piece. The status field tests are still failing but the overall result looks much better:

operator-sdk scorecard
INFO[0000] Using config file: /home/andrew/workspace/container-jfr-operator/.osdk-scorecard.yaml 
basic:
	Spec Block Exists                   : pass
	Labels: 
		"necessity":"required"
		"suite":"basic"
		"test":"checkspectest"

	Status Block Exists                 : fail
	Labels: 
		"test":"checkstatustest"
		"necessity":"required"
		"suite":"basic"
	Suggestions: 
		Add a 'status' field to your Custom Resource
		Add a 'status' field to your Custom Resource

	Writing into CRs has an effect      : pass
	Labels: 
		"test":"writingintocrshaseffecttest"
		"necessity":"required"
		"suite":"basic"

olm:
	Spec fields with descriptors        : fail
	Labels: 
		"necessity":"required"
		"suite":"olm"
		"test":"specdescriptorstest"
	Suggestions: 
		Add a spec descriptor for minimal
		Add a spec descriptor for port
		Add a spec descriptor for recordingRequests

	Status fields with descriptors      : pass
	Labels: 
		"test":"statusdescriptorstest"
		"necessity":"required"
		"suite":"olm"

	Bundle Validation Test              : pass
	Labels: 
		"suite":"olm"
		"test":"bundlevalidationtest"
		"necessity":"required"
	Log:
		time="2020-02-24T12:09:13-05:00" level=info msg="loading Bundles" dir=deploy/olm-catalog/container-jfr-operator-bundle
time="2020-02-24T12:09:13-05:00" level=info msg=directory dir=deploy/olm-catalog/container-jfr-operator-bundle file=container-jfr-operator-bundle load=bundles
time="2020-02-24T12:09:13-05:00" level=info msg=directory dir=deploy/olm-catalog/container-jfr-operator-bundle file=0.3.0 load=bundles
time="2020-02-24T12:09:13-05:00" level=info msg="found csv, loading bundle" dir=deploy/olm-catalog/container-jfr-operator-bundle file=container-jfr-operator-bundle.v0.3.0.clusterserviceversion.yaml load=bundles
time="2020-02-24T12:09:13-05:00" level=info msg="loading bundle file" dir=deploy/olm-catalog/container-jfr-operator-bundle/0.3.0 file=container-jfr-operator-bundle.v0.3.0.clusterserviceversion.yaml load=bundle
time="2020-02-24T12:09:13-05:00" level=info msg="loading bundle file" dir=deploy/olm-catalog/container-jfr-operator-bundle/0.3.0 file=containerjfrs.rhjmc.redhat.com.crd.yaml load=bundle
time="2020-02-24T12:09:13-05:00" level=info msg="loading bundle file" dir=deploy/olm-catalog/container-jfr-operator-bundle/0.3.0 file=flightrecorders.rhjmc.redhat.com.crd.yaml load=bundle
time="2020-02-24T12:09:13-05:00" level=info msg="loading Packages and Entries" dir=deploy/olm-catalog/container-jfr-operator-bundle
time="2020-02-24T12:09:13-05:00" level=info msg=directory dir=deploy/olm-catalog/container-jfr-operator-bundle file=container-jfr-operator-bundle load=package
time="2020-02-24T12:09:13-05:00" level=info msg=directory dir=deploy/olm-catalog/container-jfr-operator-bundle file=0.3.0 load=package


	Provided APIs have validation       : pass
	Labels: 
		"test":"crdshavevalidationtest"
		"necessity":"required"
		"suite":"olm"

	Owned CRDs have resources listed    : pass
	Labels: 
		"necessity":"required"
		"suite":"olm"
		"test":"crdshaveresourcestest"
	Suggestions: 
		If it would be helpful to an end-user to understand or troubleshoot your CR, consider adding resources [persistentvolumeclaims/v1 containerjfrs/v1alpha1 flightrecorders/v1alpha1 servicemonitors/v1 routes/v1] to the resources section for owned CRD ContainerJFR
		If it would be helpful to an end-user to understand or troubleshoot your CR, consider adding resources [servicemonitors/v1 flightrecorders/v1alpha1 containerjfrs/v1alpha1] to the resources section for owned CRD FlightRecorder


make: *** [Makefile:79: scorecard] Error 1

@andrewazores
Copy link
Member Author

@ebaron do you think this can be merged without fixes for the scorecard test results above? I've spent some time poking at them so far but haven't figured out what exactly is expected yet. In any case, it seems like they're only tangentially related to the SDK upgrade.

I'll do a last double-check smoke test that everything still works as expected with the last few changes I made, but nothing has changed significantly since end of last week when the bundling and index pieces got put in.

@andrewazores andrewazores changed the title [WIP] Update operator-sdk version Update operator-sdk version Feb 25, 2020
@andrewazores
Copy link
Member Author

Rebased.

Copy link
Member

@ebaron ebaron left a comment

Choose a reason for hiding this comment

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

Looks good, just a couple comments below.

Makefile Outdated Show resolved Hide resolved
pkg/apis/rhjmc/v1alpha1/zz_generated.openapi.go Outdated Show resolved Hide resolved
Copy link
Member

@ebaron ebaron left a comment

Choose a reason for hiding this comment

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

Good to go from my end. Nice work!

@andrewazores andrewazores merged commit d6486ea into cryostatio:master Feb 25, 2020
@andrewazores andrewazores deleted the update-operator-sdk branch February 25, 2020 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants