updates for 1.4 compat #314

weshayutin · 2025-09-17T20:06:43Z

Why the changes were made

Error running the latest oadp-non-admin controller w/ oadp.14

2025-09-17T18:33:29.871Z	INFO	Starting workers	{"controller": "nonadminbackupstoragelocation", "controllerGroup": "oadp.openshift.io", "controllerKind": "NonAdminBackupStorageLocation", "worker count": 1}
2025-09-17T18:36:36.103Z	INFO	VeleroBackup with label not found, creating one	{"controller": "nonadminbackup", "controllerGroup": "oadp.openshift.io", "controllerKind": "NonAdminBackup", "NonAdminBackup": {"name":"nacuser1-backup-enforcement-dm-1","namespace":"nacuser1"}, "namespace": "nacuser1", "name": "nacuser1-backup-enforcement-dm-1", "reconcileID": "66a7acd9-2e56-48f2-afce-72cd36a505a6", "UUID": "nacuser1-nacuser1-backup-e-b9a51801-1db9-4e31-af75-201bf29a1b45"}
2025-09-17T18:36:36.110Z	INFO	VeleroBackup successfully created	{"controller": "nonadminbackup", "controllerGroup": "oadp.openshift.io", "controllerKind": "NonAdminBackup", "NonAdminBackup": {"name":"nacuser1-backup-enforcement-dm-1","namespace":"nacuser1"}, "namespace": "nacuser1", "name": "nacuser1-backup-enforcement-dm-1", "reconcileID": "66a7acd9-2e56-48f2-afce-72cd36a505a6"}
2025-09-17T18:36:43.331Z	ERROR	Failed to update NonAdminBackup Status	{"controller": "nonadminbackup", "controllerGroup": "oadp.openshift.io", "controllerKind": "NonAdminBackup", "NonAdminBackup": {"name":"nacuser1-backup-enforcement-dm-1","namespace":"nacuser1"}, "namespace": "nacuser1", "name": "nacuser1-backup-enforcement-dm-1", "reconcileID": "9a81bf6f-f365-4ad8-9a54-56ea94a4af8a", "error": "Operation cannot be fulfilled on nonadminbackups.oadp.openshift.io \"nacuser1-backup-enforcement-dm-1\": the object has been modified; please apply your changes to the latest version and try again"}
github.com/migtools/oadp-non-admin/internal/controller.(*NonAdminBackupReconciler).createVeleroBackupAndSyncWithNonAdminBackup
	/workspace/internal/controller/nonadminbackup_controller.go:802
github.com/migtools/oadp-non-admin/internal/controller.(*NonAdminBackupReconciler).Reconcile
	/workspace/internal/controller/nonadminbackup_controller.go:162
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).Reconcile
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.19.3/pkg/internal/controller/controller.go:116
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).reconcileHandler
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.19.3/pkg/internal/controller/controller.go:303
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).processNextWorkItem
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.19.3/pkg/internal/controller/controller.go:263
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).Start.func2.2
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.19.3/pkg/internal/controller/controller.go:224
2025-09-17T18:36:43.331Z	ERROR	Reconciler error	{"controller": "nonadminbackup", "controllerGroup": "oadp.openshift.io", "controllerKind": "NonAdminBackup", "NonAdminBackup": {"name":"nacuser1-backup-enforcement-dm-1","namespace":"nacuser1"}, "namespace": "nacuser1", "name": "nacuser1-backup-enforcement-dm-1", "reconcileID": "9a81bf6f-f365-4ad8-9a54-56ea94a4af8a", "error": "Operation cannot be fulfilled on nonadminbackups.oadp.openshift.io \"nacuser1-backup-enforcement-dm-1\": the object has been modified; please apply your changes to the latest version and try again"}
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).reconcileHandler
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.19.3/pkg/internal/controller/controller.go:316
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).processNextWorkItem
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.19.3/pkg/internal/controller/controller.go:263
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).Start.func2.2
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.19.3/pkg/internal/controller/controller.go:224

How to test the changes made

build and override and test

Signed-off-by: Wesley Hayutin <weshayutin@gmail.com>

oadp-snyk · 2025-09-17T20:06:53Z

⛔ Snyk checks have failed. 2 issues have been found so far.

Icon	Severity	Issues
	Critical	0
	High	0
	Medium	2
	Low	0

⛔ security/snyk check is complete. 2 issues have been found. (View Details)

✅ license/snyk check is complete. No issues have been found. (View Details)

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

mpryc · 2025-09-17T21:03:48Z

/lgtm

Do you have two controllers running? One deployed by dpa (pod in openshift-adp) and second via make run from development environment?

Anyway the retry is always safe to perform imo.

shubham-pampattiwar · 2025-09-17T21:06:05Z

Are we seeing this only on 1.4 ? or this visible on main/1.5 ? If yes, I would post a fix to main first and then CP.

mpryc · 2025-09-17T21:13:27Z

tide requires: openshift/release#69404

mpryc · 2025-09-17T21:14:31Z

/cherry-pick oadp-1.5

openshift-cherrypick-robot · 2025-09-17T21:14:35Z

@mpryc: once the present PR merges, I will cherry-pick it on top of oadp-1.5 in a new PR and assign it to you.

In response to this:

/cherry-pick oadp-1.5

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-sigs/prow repository.

mpryc · 2025-09-17T21:14:41Z

/cherry-pick main

openshift-cherrypick-robot · 2025-09-17T21:14:44Z

@mpryc: once the present PR merges, I will cherry-pick it on top of main in a new PR and assign it to you.

In response to this:

/cherry-pick main

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-sigs/prow repository.

weshayutin · 2025-09-17T22:46:31Z

@shubham-pampattiwar @mpryc I need to test this a bit more.. AFAIK this is ONLY 1.4 issue and caused by diff controller versions. DO NOT CHERRY PICK.. No NEED to RUSH

openshift-ci · 2025-09-17T23:10:12Z

New changes are detected. LGTM label has been removed.

weshayutin · 2025-09-17T23:39:11Z

@shubham-pampattiwar @mpryc ok.. I don't I can get this working w/ go 1.20. I could be just silly/stupid but I will need your advice here. Could we essentially ship the oadp-1.5 version w/ 1.4 and just fix that conflict? I don't know what to do :) SUPRISE!

Signed-off-by: Michal Pryc <mpryc@redhat.com>

mpryc · 2025-09-18T08:12:15Z

@weshayutin it's the CONTROLLER_TOOLS_VERSION specified in the Makefile incompatible with newer go version.

There is no need to downgrade go builder, even velero oadp-1.4 has 1.23:
https://github.com/openshift/velero/blob/oadp-1.4/go.mod#L3C1-L3C10

Most importantly we need to ensure the controller-runtime used in oadp-1.4 matches the version shipped with OpenShift that is using oadp-1.4, we can see that we are using for velero same:
https://github.com/openshift/velero/blob/58873ca30add4789ff65dfdc022d768db577fb86/go.mod#L61

I think this is correct way to go:
weshayutin#1

In the above PR note the pointer to your fork of oadp-operator, which would need to be replaced once draft is merged to openshift/oadp-operator:
https://github.com/weshayutin/oadp-non-admin/pull/1/files#diff-33ef32bf6c23acb95f5902d7097b7a1d5128ca061167ec0716715b0b9eeaa5f6R173

$ make build
migtools-oadp-non-admin/bin/controller-gen-v0.16.5 rbac:roleName=non-admin-controller-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
migtools-oadp-non-admin/bin/controller-gen-v0.16.5 object:headerFile="hack/boilerplate.go.txt" paths="./..."
go fmt ./...
go vet ./...
go build -o bin/manager cmd/main.go

$ echo $?
0

Fixes to build with oadp-1.4

weshayutin · 2025-09-19T14:54:57Z

@shubham-pampattiwar we need some more work done here to avoid conflicts. Now that the controller-gen is downgraded, we're still seeing some issues.

Modifications which allows to remove race condition in the Status and Finalizer updates. Fixed race condition in Status update. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Michal Pryc <mpryc@redhat.com>

mpryc · 2025-09-19T21:53:53Z

@shubham-pampattiwar we need some more work done here to avoid conflicts. Now that the controller-gen is downgraded, we're still seeing some issues.

Have a nice weekend :) - fixes all tests:
weshayutin#2

Fixes of the Status.Update() calls and use Patch for finalizer

weshayutin · 2025-09-19T22:29:56Z

 make test
/home/whayutin/OPENSHIFT/git/OADP/oadp-non-admin/bin/controller-gen-v0.16.5 rbac:roleName=non-admin-controller-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
/home/whayutin/OPENSHIFT/git/OADP/oadp-non-admin/bin/controller-gen-v0.16.5 object:headerFile="hack/boilerplate.go.txt" paths="./..."
go fmt ./...
go vet ./...
KUBEBUILDER_ASSETS="/home/whayutin/OPENSHIFT/git/OADP/oadp-non-admin/bin/k8s/1.29.3-linux-amd64" go test $(go list ./... | grep -v /e2e) -coverprofile cover.out
	github.com/migtools/oadp-non-admin/api/v1alpha1		coverage: 0.0% of statements
?   	github.com/migtools/oadp-non-admin/internal/common/constant	[no test files]
ok  	github.com/migtools/oadp-non-admin/cmd	0.023s	coverage: 14.0% of statements
ok  	github.com/migtools/oadp-non-admin/internal/common/function	0.031s	coverage: 78.4% of statements
	github.com/migtools/oadp-non-admin/test/utils		coverage: 0.0% of statements
	github.com/migtools/oadp-non-admin/internal/source		coverage: 0.0% of statements
	github.com/migtools/oadp-non-admin/internal/predicate		coverage: 0.0% of statements
	github.com/migtools/oadp-non-admin/internal/handler		coverage: 0.0% of statements
ok  	github.com/migtools/oadp-non-admin/internal/controller	92.401s	coverage: 67.2% of statements
whayutin@fedora:~/OPENSHIFT/git/OADP/oadp-non-admin$ git branch
  foo
  oadp-dev
* oadp_14_compat

weshayutin · 2025-09-19T22:30:05Z

thanks @mpryc

mpryc · 2025-09-22T17:56:24Z

/override snyk

openshift-ci · 2025-09-22T17:56:28Z

@mpryc: /override requires failed status contexts, check run or a prowjob name to operate on.
The following unknown contexts/checkruns were given:

snyk

Only the following failed contexts/checkruns were expected:

ci/prow/images
license/snyk ( Hybrid Platforms - OpenShift API for Data Protection)
pull-ci-migtools-oadp-non-admin-master-images
security/snyk ( Hybrid Platforms - OpenShift API for Data Protection)
tide

If you are trying to override a checkrun that has a space in it, you must put a double quote on the context.

In response to this:

/override snyk

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-sigs/prow repository.

mpryc · 2025-09-22T17:56:53Z

/override security/snyk

openshift-ci · 2025-09-22T17:56:57Z

@mpryc: /override requires failed status contexts, check run or a prowjob name to operate on.
The following unknown contexts/checkruns were given:

security/snyk

Only the following failed contexts/checkruns were expected:

ci/prow/images
license/snyk ( Hybrid Platforms - OpenShift API for Data Protection)
pull-ci-migtools-oadp-non-admin-master-images
security/snyk ( Hybrid Platforms - OpenShift API for Data Protection)
tide

If you are trying to override a checkrun that has a space in it, you must put a double quote on the context.

In response to this:

/override security/snyk

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-sigs/prow repository.

weshayutin · 2025-09-22T18:51:01Z

/override security/snyk

openshift-ci · 2025-09-22T18:51:05Z

@weshayutin: /override requires failed status contexts, check run or a prowjob name to operate on.
The following unknown contexts/checkruns were given:

security/snyk

Only the following failed contexts/checkruns were expected:

ci/prow/images
license/snyk ( Hybrid Platforms - OpenShift API for Data Protection)
pull-ci-migtools-oadp-non-admin-master-images
security/snyk ( Hybrid Platforms - OpenShift API for Data Protection)
tide

If you are trying to override a checkrun that has a space in it, you must put a double quote on the context.

In response to this:

/override security/snyk

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-sigs/prow repository.

mpryc

/lgtm

mpryc · 2025-09-22T18:56:35Z

/override security/snyk

openshift-ci · 2025-09-22T18:56:39Z

@mpryc: /override requires failed status contexts, check run or a prowjob name to operate on.
The following unknown contexts/checkruns were given:

security/snyk

Only the following failed contexts/checkruns were expected:

ci/prow/images
license/snyk ( Hybrid Platforms - OpenShift API for Data Protection)
pull-ci-migtools-oadp-non-admin-master-images
security/snyk ( Hybrid Platforms - OpenShift API for Data Protection)
tide

If you are trying to override a checkrun that has a space in it, you must put a double quote on the context.

In response to this:

/override security/snyk

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-sigs/prow repository.

openshift-ci · 2025-09-22T18:59:43Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mpryc, shubham-pampattiwar, weshayutin

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~~ [mpryc,shubham-pampattiwar]

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

openshift-cherrypick-robot · 2025-09-22T19:59:03Z

@mpryc: new pull request created: #319

In response to this:

/cherry-pick oadp-1.5

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-sigs/prow repository.

openshift-cherrypick-robot · 2025-09-22T19:59:40Z

@mpryc: cannot checkout main: error checking out "main": exit status 1 error: pathspec 'main' did not match any file(s) known to git

In response to this:

/cherry-pick main

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-sigs/prow repository.

updates for 1.4 compat

e79221c

Signed-off-by: Wesley Hayutin <weshayutin@gmail.com>

openshift-ci bot requested review from mrnold and shubham-pampattiwar September 17, 2025 20:06

weshayutin mentioned this pull request Sep 17, 2025

oadp 1.4 onboard nac openshift/oadp-operator#1957

Closed

openshift-ci bot assigned mpryc Sep 17, 2025

openshift-ci bot added the lgtm label Sep 17, 2025

mpryc previously approved these changes Sep 17, 2025

View reviewed changes

openshift-ci bot added the approved label Sep 17, 2025

mpryc mentioned this pull request Sep 17, 2025

Add tide for the oadp-1.4 branch for oadp-non-admin openshift/release#69404

Merged

weshayutin added the do-not-merge/hold label Sep 17, 2025

ensure go mod is built for 1.4

4897807

weshayutin dismissed mpryc’s stale review via 4897807 September 17, 2025 23:10

openshift-ci bot removed the lgtm label Sep 17, 2025

openshift-ci bot removed the approved label Sep 17, 2025

Fixes to build with oadp-1.4

69a7d57

Signed-off-by: Michal Pryc <mpryc@redhat.com>

Merge pull request #1 from mpryc/oadp_14_compat-fixes

f7baca7

Fixes to build with oadp-1.4

weshayutin removed the do-not-merge/hold label Sep 18, 2025

Merge branch 'oadp-1.4' into oadp_14_compat

e3f8d1a

Fixes of the Status.Update() calls and use Patch for finalizer

84b4e2f

Modifications which allows to remove race condition in the Status and Finalizer updates. Fixed race condition in Status update. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Michal Pryc <mpryc@redhat.com>

Merge pull request #2 from mpryc/oadp_14_compat

1000274

Fixes of the Status.Update() calls and use Patch for finalizer

mpryc previously approved these changes Sep 22, 2025

View reviewed changes

openshift-ci bot added the approved label Sep 22, 2025

shubham-pampattiwar previously approved these changes Sep 22, 2025

View reviewed changes

weshayutin dismissed stale reviews from shubham-pampattiwar and mpryc via 1000274 September 22, 2025 18:49

weshayutin force-pushed the oadp_14_compat branch from a2ca0b0 to 1000274 Compare September 22, 2025 18:49

openshift-ci bot removed the approved label Sep 22, 2025

weshayutin added approved lgtm labels Sep 22, 2025

mpryc approved these changes Sep 22, 2025

View reviewed changes

shubham-pampattiwar approved these changes Sep 22, 2025

View reviewed changes

weshayutin merged commit 64e3dd7 into migtools:oadp-1.4 Sep 22, 2025
3 of 4 checks passed

openshift-cherrypick-robot mentioned this pull request Sep 22, 2025

[oadp-1.5] updates for 1.4 compat #319

Open

updates for 1.4 compat #314

updates for 1.4 compat #314

Uh oh!

Conversation

weshayutin commented Sep 17, 2025

Why the changes were made

How to test the changes made

Uh oh!

oadp-snyk commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⛔ Snyk checks have failed. 2 issues have been found so far.

Uh oh!

mpryc commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shubham-pampattiwar commented Sep 17, 2025

Uh oh!

mpryc commented Sep 17, 2025

Uh oh!

mpryc commented Sep 17, 2025

Uh oh!

openshift-cherrypick-robot commented Sep 17, 2025

Uh oh!

mpryc commented Sep 17, 2025

Uh oh!

openshift-cherrypick-robot commented Sep 17, 2025

Uh oh!

weshayutin commented Sep 17, 2025

Uh oh!

openshift-ci bot commented Sep 17, 2025

Uh oh!

weshayutin commented Sep 17, 2025

Uh oh!

mpryc commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

weshayutin commented Sep 19, 2025

Uh oh!

mpryc commented Sep 19, 2025

Uh oh!

weshayutin commented Sep 19, 2025

Uh oh!

weshayutin commented Sep 19, 2025

Uh oh!

mpryc commented Sep 22, 2025

Uh oh!

openshift-ci bot commented Sep 22, 2025

Uh oh!

mpryc commented Sep 22, 2025

Uh oh!

openshift-ci bot commented Sep 22, 2025

Uh oh!

weshayutin commented Sep 22, 2025

Uh oh!

openshift-ci bot commented Sep 22, 2025

Uh oh!

mpryc left a comment

Choose a reason for hiding this comment

Uh oh!

mpryc commented Sep 22, 2025

Uh oh!

openshift-ci bot commented Sep 22, 2025

Uh oh!

openshift-ci bot commented Sep 22, 2025

Uh oh!

Uh oh!

openshift-cherrypick-robot commented Sep 22, 2025

Uh oh!

openshift-cherrypick-robot commented Sep 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

oadp-snyk commented Sep 17, 2025 •

edited

Loading

mpryc commented Sep 17, 2025 •

edited

Loading

mpryc commented Sep 18, 2025 •

edited

Loading