Closed hubeadmin closed 4 years ago
/retest
Few thoughts from a quick look:
I will try the import and review tthe changelog if I have enough time today.
Since there are no e2e, please provide Verification steps that would sufficiently verify the functionality of the operator.
The changes in deploy/olm-catalog/application-monitoring-operator/1.1.6/application-monitoring-operator.v1.1.6.clusterserviceversion.yaml
look suspicious, and it looks wrong on master also..
@pb82 do you know why it v1.1.16 CSV on master has references of v1.1.15 ?
@matskiv @HubertStefanski That seems definitely wrong. The manifest in the integreatly operator is correct though: https://github.com/integr8ly/integreatly-operator/blob/master/manifests/integreatly-monitoring/1.1.6/application-monitoring-operator.v1.1.6.clusterserviceversion.yaml
I believe this here is automatically generated, so we might need to fix those scripts. ping @austinmartinh
* maybe bump minor version, since this is a big-ish change?
I agree, we probably best to bump it to 1.2.0
- will we be able to import is as a dependency in integreatly-operator? We are only on 0.15.1 there: https://github.com/integr8ly/integreatly-operator/blob/39e2a9cb913dfea12de085f7320068ff3ec34db0/go.mod#L34 Technically we should be able to do so, having spoken with @davidffrench, although that's still to be tested locally
- @HubertStefanski have you looked at SDK changelog between old and new version? It's probably worth reviewing for breaking changes - https://github.com/operator-framework/operator-sdk/blob/master/CHANGELOG.md I have looked through them, the upgrade guide also points out a few breaking changes, which as far as I've looked into it seem to be largely unrelated for the scope of this issue
As for the changes to 1.1.6/application-monitoring-operator.v1.1.6.clusterserviceversion.yaml
Thanks for pointing that out, that change (in this PR) was probably largely due to me mindlessly generating a csv with the wrong version, I'll revert the commit that changed those specific things, and push the corrected one soon :+1:
/cc
Looks okay to me. @grdryn @pb82 any input on this PR guys?
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: grdryn, pb82
The full list of commands accepted by this bot can be found here.
The pull request process is described here
Needed for a new release to prevent pod duplication as described in https://issues.redhat.com/browse/INTLY-8046
Changes
make gen/csv
recipe to useoperator-sdk generate csv
instead ofolm-catalog
which has been deprecated.deploy/olm-catalog/application-monitoring-operator
directory, and previous versions are no longer stored in-project.Verification
To ensure the newer version of the operator-sdk works as expected
make setup/travis
make code/gen
make code/run
make image/build
make image/build/test