search

openshift / vertical-pod-autoscaler-operator

An Operator for running the Vertical Pod Autoscaler on OpenShift
Apache License 2.0
27 stars 30 forks source link

PODAUTO-89: Update repo to operator-sdk v1.35 #169

Closed maxcao13 closed 2 months ago

maxcao13 commented 2 months ago

This PR migrates the repo to a modern operator-sdk v1.35 setup from the previous hacked together setup. This was done by first following the steps in this migration guide that John linked: https://sdk.operatorframework.io/docs/building-operators/golang/migration/, and then porting over the old repo's tests and other configuration for backwards compat.

Here's a summary of the list of changes:

This will definitely require some additional changes in Prow config and probably in here as well to make that work.

Please let me know if I missed anything :-)

openshift-ci-robot commented 2 months ago

@maxcao13: This pull request references PODAUTO-89 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.17.0" version, but no target version was set.

In response to [this](https://github.com/openshift/vertical-pod-autoscaler-operator/pull/169): >This PR migrates the repo to a modern operator-sdk v1.35 setup from the previous hacked together setup. This was done by first following the steps in this migration guide that John linked: https://sdk.operatorframework.io/docs/building-operators/golang/migration/, and then porting over the old repo's tests and other configuration for backwards compat. > >Here's a summary of the list of changes: >* Operator-sdk migration-specific > * Migrated types from `pkg/apis` to `api`. > * Migrated reconcile code to the new manager format > * Updated layout of the repo to fit the operator-sdk layout (config with Kustomize, bundle builds, etc.) > * Updated Dockerfiles > * Migrated old build and deploy steps > * There is now a simple `deploy` make target, `deploy-bundle` target, where you can deploy the operator with OLM, and a `deploy-catalog` target where you can deploy a `CatalogSource` + `OperatorGroup` with the operator contained to see the UI steps that someone may take to create the operator in the OCP console OperatorHub > * full list can be found in the link above >* VPA-specific > * Removed `hack/manifest-diff.sh` and its jq helper since we no longer need to sync install manifests with our CSV (operator-sdk does it for us!) > * Updated `update-vendor.sh` to fit the new repo > * Removed `hack/go-*.sh` since operator-sdk generated Makefile has these commands already > * Updated `hack/e2e.sh` to include a flag where you can use your own temporary directory to pull vpa tests from (this was because my internet was being bad and I found it annoying to keep cloning 80 MB) > * Created `config/olm-catalog` directory to contain the yamls that are required for the catalog build > * Created `config/vpa` directory with various configs which is required for the vpa operand to function > >This will definitely require some additional changes in Prow config and probably in here as well to make that work. > >Please let me know if I missed anything :-) Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Fvertical-pod-autoscaler-operator). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
openshift-ci-robot commented 2 months ago

@maxcao13: This pull request references PODAUTO-89 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.17.0" version, but no target version was set.

In response to [this](https://github.com/openshift/vertical-pod-autoscaler-operator/pull/169): >This PR migrates the repo to a modern operator-sdk v1.35 setup from the previous hacked together setup. This was done by first following the steps in this migration guide that John linked: https://sdk.operatorframework.io/docs/building-operators/golang/migration/, and then porting over the old repo's tests and other configuration for backwards compat. > >Here's a summary of the list of changes: >* Operator-sdk migration-specific > * Migrated types from `pkg/apis` to `api`. > * Migrated reconcile code to the new manager format > * Updated layout of the repo to fit the operator-sdk layout (config with Kustomize, bundle builds, etc.) > * Updated Dockerfiles > * Migrated old build and deploy steps > * There is now a simple `deploy` make target, `deploy-bundle` target, where you can deploy the operator with OLM, and a `deploy-catalog` target where you can deploy a `CatalogSource` + `OperatorGroup` with the operator contained to see the UI steps that someone may take to create the operator in the OCP console OperatorHub > * full list can be found in the link above >* VPA-specific > * Removed `hack/manifest-diff.sh` and its jq helper since we no longer need to sync install manifests with our CSV (operator-sdk does it for us!) > * Updated `update-vendor.sh` to fit the new repo > * Removed `hack/go-*.sh` since operator-sdk generated Makefile has these commands already > * Updated `hack/e2e.sh` to include a flag where you can use your own temporary directory to pull vpa tests from (this was because my internet was being bad and I found it annoying to keep cloning 80 MB) > * Created `config/olm-catalog` directory to contain the yamls that are required for the catalog build > * Created `config/vpa` directory with various configs which is required for the vpa operand to function > >This will definitely require some additional changes in Prow config and probably in here as well to make that work. > >Please let me know if I missed anything :-) Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Fvertical-pod-autoscaler-operator). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
maxcao13 commented 2 months ago

Needs some additional fixes that I haven't pushed yet, locally e2e tests look good! image

maxcao13 commented 2 months ago

Another commit is just going to be a README update, which can detail some of the testing steps anyone can take to review.

maxcao13 commented 2 months ago

My gitignore was seriously ignoring vendored directories and I didn't notice... :/

maxcao13 commented 2 months ago

Reherasals have passed here: https://github.com/openshift/release/pull/55425

openshift-ci[bot] commented 2 months ago

@maxcao13: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/golint 86664a44b1ec3f30d11a9e61ca674860818c0df8 link true /test golint

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
maxcao13 commented 2 months ago

Not really sure why golint still exists since I replaced it with lint. I'm gonna assume it's stale since we changed CI in the middle of this PR.

Also, lint is expected to fail because of that one deprecated function I mentioned earlier. :-) Link to failed test

jkyros commented 2 months ago

Also, lint is expected to fail because of that one deprecated function I mentioned earlier. :-) Link to failed test

If we're squeamish about possible consequences from something like e.g. https://github.com/jkyros/vertical-pod-autoscaler-operator/commit/849ad7eebd82ef187f3056dcff0aa276d4af172e to fix the lint deprecation, can we maybe at least tag that line with a TODO comment and a //nolint: staticcheck so we don't have to skip the test in the mean time while we fix it?

jkyros commented 2 months ago

Tests passed - awesome! Thanks for all your hard work here Max! /lgtm

openshift-ci[bot] commented 2 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jkyros, maxcao13

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/openshift/vertical-pod-autoscaler-operator/blob/master/OWNERS)~~ [jkyros] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment