Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

bump AppWrapper to 1.0.0 #652

Merged
merged 7 commits into from
Feb 4, 2025
Merged

bump AppWrapper to 1.0.0 #652

merged 7 commits into from
Feb 4, 2025

Conversation

dgrove-oss
Copy link
Collaborator

  • Update to Kueue 0.10
  • Remove deprecated kube-rbac-proxy
  • Refactor workload controller to prepare to upstream to Kueue

Combines go.mod changes of #651 with the Makefile change to pick up the CRD.

@openshift-ci openshift-ci bot requested review from astefanutti and dimakis January 16, 2025 18:55
@ChristianZaccaria
Copy link
Contributor

Perhaps the Makefile can also be updated to use Kueue v0.10.0

@ChristianZaccaria
Copy link
Contributor

I'm getting this error on attempt to create AppWrapper, in the CFO logs:

2025-01-16T20:51:21Z ERROR Unhandled Error {"logger": "UnhandledError", "error": "pkg/mod/k8s.io/client-go@v0.31.4/tools/cache/reflector.go:243: Failed to watch *v1.Namespace: failed to list *v1.Namespace: namespaces is forbidden: User "system:serviceaccount:redhat-ods-applications:codeflare-operator-controller-manager" cannot list resource "namespaces" in API group "" at the cluster scope"}

@dgrove-oss dgrove-oss force-pushed the awbump branch 2 times, most recently from 115562f to 1c744d3 Compare January 22, 2025 14:02
@dgrove-oss
Copy link
Collaborator Author

I rebased to pick up refreshed go.mod from #655. Building the binary locally is fine.

We'll need to update the go-toolset image in the Dockerfile to something that supports go 1.23 before this can be merged. Otherwise I think it should be ready to go.

@dgrove-oss dgrove-oss mentioned this pull request Jan 22, 2025
Closed
4 tasks
@dgrove-oss dgrove-oss force-pushed the awbump branch 2 times, most recently from d2a00b6 to a558ce5 Compare January 27, 2025 18:39
sutaakar
sutaakar reviewed Jan 28, 2025
View reviewed changes
Dockerfile Outdated Show resolved Hide resolved
@dgrove-oss
Copy link
Collaborator Author

Should be good to go when you are ready to merge.

ChristianZaccaria
ChristianZaccaria reviewed Jan 30, 2025
View reviewed changes
Copy link
Contributor

@ChristianZaccaria ChristianZaccaria left a comment

Choose a reason for hiding this comment

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

/lgtm
Thanks!!

Kueue upgrade has been made. We will sync changes to RHOAI as soon as this PR is merged and we sync to downstream as well.

@openshift-ci openshift-ci bot added the lgtm label Jan 30, 2025
Ygnas
Ygnas reviewed Jan 30, 2025
View reviewed changes
Copy link
Contributor

@Ygnas Ygnas left a comment

Choose a reason for hiding this comment

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

/lgtm
I have tested integration with Kueue v0.10.1 and this version of the CFO.

@sutaakar
Copy link
Contributor

sutaakar commented Feb 3, 2025

@dgrove-oss Can you please rebase?

@openshift-ci openshift-ci bot removed the lgtm label Feb 3, 2025
@dgrove-oss
Copy link
Collaborator Author

rebased.

sutaakar
sutaakar reviewed Feb 3, 2025
View reviewed changes
Copy link
Contributor

@sutaakar sutaakar 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 Feb 3, 2025
ChristianZaccaria
ChristianZaccaria reviewed Feb 4, 2025
View reviewed changes
Copy link
Contributor

@ChristianZaccaria ChristianZaccaria left a comment

Choose a reason for hiding this comment

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

/lgtm thank you!!

@sutaakar
Copy link
Contributor

sutaakar commented Feb 4, 2025

/approve

@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Feb 4, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sutaakar

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:

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

@openshift-ci openshift-ci bot added the approved label Feb 4, 2025
@openshift-merge-bot openshift-merge-bot bot merged commit ef70cf9 into project-codeflare:main Feb 4, 2025
8 checks passed
@dgrove-oss dgrove-oss deleted the awbump branch February 4, 2025 13:51
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@sutaakar sutaakar sutaakar left review comments

@Ygnas Ygnas Ygnas left review comments

@ChristianZaccaria ChristianZaccaria ChristianZaccaria left review comments

@astefanutti astefanutti Awaiting requested review from astefanutti

@dimakis dimakis Awaiting requested review from dimakis

Assignees

@sutaakar sutaakar

@Ygnas Ygnas

@ChristianZaccaria ChristianZaccaria

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@dgrove-oss @ChristianZaccaria @sutaakar @Ygnas @openshift-merge-robot