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

Updates for operator sdk v1.28 #45

Merged
merged 31 commits into from
Aug 31, 2023

Conversation

michael-valdron
Copy link
Member

@michael-valdron michael-valdron commented Aug 24, 2023

Please specify the area for this PR

What does does this PR do / why we need it:

Provides updates to the project files to support Operator SDK version 1.28, these changes are documented under https://sdk.operatorframework.io/docs/upgrading-sdk-version/ up to v1.28 docs.

Additional Changes:

  • OpenShift Integration test script deploys 15 seconds before running test suite
  • Integration test suite uses registry-operator-system for registry operator pod instead of TEST_NAMESPACE
    • This fixes the ability to use TEST_NAMESPACE as an optional separate namespace for the devfile registry test deployments

Which issue(s) this PR fixes:

Fixes #?

fixes devfile/api#881
fixes devfile/api#1106

PR acceptance criteria:

  • Test Coverage
    • Are your changes sufficiently tested, and are any applicable test cases added or updated to cover your changes?
  • Gosec scans

Documentation

How to test changes / Special notes to the reviewer:

@openshift-ci
Copy link

openshift-ci bot commented Aug 24, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@codecov
Copy link

codecov bot commented Aug 27, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.30% 🎉

Comparison is base (9ac9961) 22.41% compared to head (ff9e578) 22.72%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #45      +/-   ##
==========================================
+ Coverage   22.41%   22.72%   +0.30%     
==========================================
  Files          23       23              
  Lines        1307     1307              
==========================================
+ Hits          293      297       +4     
+ Misses        998      995       -3     
+ Partials       16       15       -1     
Files Changed Coverage Δ
api/v1alpha1/zz_generated.deepcopy.go 36.28% <ø> (ø)
...rollers/clusterdevfileregistrieslist_controller.go 92.85% <ø> (ø)
controllers/devfileregistrieslist_controller.go 92.85% <ø> (+14.28%) ⬆️
controllers/validate.go 100.00% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@johnmcollier johnmcollier left a comment

Choose a reason for hiding this comment

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

Left some comments.

Also, could we move back the controller go sources from internal/controller/xyz to just controllers/? I think internal/controller is the default for new controller runtime projects, but the added "internal" in the path is unnecessary for our use case IMO

CHANGELOG.md Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
config/rbac/service_account.yaml Outdated Show resolved Hide resolved
@michael-valdron
Copy link
Member Author

@michael-valdron: 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/v4.12-registry-operator-integration-test 1f0abf2 link true /test v4.12-registry-operator-integration-test

Full PR test history. Your PR dashboard.

CI is still running the integration tests using go 1.18, this needs to be updated to go 1.19 for this to pass.

go: go.mod file indicates go 1.19, but maximum version supported by tidy is 1.18

@michael-valdron
Copy link
Member Author

Left some comments.

Also, could we move back the controller go sources from internal/controller/xyz to just controllers/? I think internal/controller is the default for new controller runtime projects, but the added "internal" in the path is unnecessary for our use case IMO

Reverted go v4 project structure changes.

@michael-valdron
Copy link
Member Author

/retest

@michael-valdron
Copy link
Member Author

/retest-required

1 similar comment
@michael-valdron
Copy link
Member Author

/retest-required

@michael-valdron
Copy link
Member Author

/retest-required

@michael-valdron
Copy link
Member Author

/retest

@michael-valdron michael-valdron force-pushed the chore/operator-sdk-sync branch 3 times, most recently from 1f2e22b to a5e3d53 Compare August 30, 2023 05:59
@michael-valdron
Copy link
Member Author

/test

@openshift-ci
Copy link

openshift-ci bot commented Aug 30, 2023

@michael-valdron: The /test command needs one or more targets.
The following commands are available to trigger required jobs:

  • /test v4.12-images
  • /test v4.12-registry-operator-integration-test

Use /test all to run all jobs.

In response to this:

/test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@michael-valdron
Copy link
Member Author

/test all

@michael-valdron
Copy link
Member Author

/retest

@michael-valdron
Copy link
Member Author

/retest

@johnmcollier
Copy link
Member

/retest

@openshift-ci openshift-ci bot added the lgtm label Aug 30, 2023
@michael-valdron
Copy link
Member Author

/retest

@openshift-ci openshift-ci bot removed the lgtm label Aug 31, 2023
Copy link
Member

@johnmcollier johnmcollier left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Aug 31, 2023
@openshift-ci
Copy link

openshift-ci bot commented Aug 31, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johnmcollier, michael-valdron

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 [johnmcollier,michael-valdron]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@michael-valdron michael-valdron merged commit fd4aacc into devfile:main Aug 31, 2023
7 checks passed
thepetk pushed a commit to thepetk/devfile-registry-operator that referenced this pull request Aug 20, 2024
* required changes to update operator sdk

Signed-off-by: Michael Valdron <[email protected]>

* ginkgo v2 migration changes.

Signed-off-by: Michael Valdron <[email protected]>

* remove extra ENVTEST definition

Signed-off-by: Michael Valdron <[email protected]>

* dependencies updated to match operator sdk v1.28 requirements

Signed-off-by: Michael Valdron <[email protected]>

* refactoring controllers package to internal/controller package as part of go/v4 prep changes in operator sdk v1.28

Signed-off-by: Michael Valdron <[email protected]>

* bump gcr.io/kubebuilder/kube-rbac-proxy from v0.13.0 to v0.13.1 as part of operator sdk v1.28 changes

Signed-off-by: Michael Valdron <[email protected]>

* refactoring main.go to cmd/main.go as part of go/v4 prep changes in operator sdk v1.28

Signed-off-by: Michael Valdron <[email protected]>

* update header under integration test source

Signed-off-by: Michael Valdron <[email protected]>

* update go version from 1.18 to 1.19 under ci workflows

Signed-off-by: Michael Valdron <[email protected]>

* import admissionv1beta1 changed to admissionv1 as part of operator sdk v1.28 changes

Signed-off-by: Michael Valdron <[email protected]>

* update license header and boilerplate files

Signed-off-by: Michael Valdron <[email protected]>

* rbac service account role bindings fixed with labels added

Signed-off-by: Michael Valdron <[email protected]>

* security context for manager container updated.

Signed-off-by: Michael Valdron <[email protected]>

* recent changes applied to bundle manifests

Signed-off-by: Michael Valdron <[email protected]>

* service account renamed to fix name prefix issue.

Signed-off-by: Michael Valdron <[email protected]>

* fixed envtest binary name

Signed-off-by: Michael Valdron <[email protected]>

* fixed relative path to 'config/crd/bases' under controller test suite

Signed-off-by: Michael Valdron <[email protected]>

* controller-gen deepcopy source update to header

Signed-off-by: Michael Valdron <[email protected]>

* fixup docker-buildx rule

Signed-off-by: Michael Valdron <[email protected]>

* CHANGELOG added

Signed-off-by: Michael Valdron <[email protected]>

* included change log documenting as part of PR acceptance criteria

Signed-off-by: Michael Valdron <[email protected]>

* target os is always linux

Signed-off-by: Michael Valdron <[email protected]>

* sa changed to service-account

Signed-off-by: Michael Valdron <[email protected]>

* bundle csv update

Signed-off-by: Michael Valdron <[email protected]>

* go v4 project structure changes reverted.

Signed-off-by: Michael Valdron <[email protected]>

* restore unset GOFLAGS

Signed-off-by: Michael Valdron <[email protected]>

* use operator namespace when waiting for operator pod to run for integration test suite

Signed-off-by: Michael Valdron <[email protected]>

* add registry operator deploy command to openshift integration test script

Signed-off-by: Michael Valdron <[email protected]>

* add touch command in Makefile to create junit report if does not exist

Signed-off-by: Michael Valdron <[email protected]>

* add docker-buildx optional step to build and run the operator instruction under CONTRIBUTING.md

Signed-off-by: Michael Valdron <[email protected]>

* make /tmp/artifacts directory if does not exist before running integration tests

Signed-off-by: Michael Valdron <[email protected]>

---------

Signed-off-by: Michael Valdron <[email protected]>
Signed-off-by: thepetk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Registry operator should be in sync with operator SDK releases Update Ginkgo version in registry operator
2 participants