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

ci: Add missing CI for feast-operator image builds #4184

Merged

Conversation

tchughesiv
Copy link
Contributor

@tchughesiv tchughesiv commented May 7, 2024

What this PR does / why we need it:

Missing CI related to #4145

This change will require that the following image repos are created (if they don't already exist) -

gcr.io/kf-feast/feast-operator
gcr.io/kf-feast/feature-server

Which issue(s) this PR fixes:

#4144

@tchughesiv tchughesiv changed the title add missing CI for feast-operator image builds fix: add missing CI for feast-operator image builds May 7, 2024
@tchughesiv tchughesiv changed the title fix: add missing CI for feast-operator image builds fix: Add missing CI for feast-operator image builds May 7, 2024
@tchughesiv tchughesiv changed the title fix: Add missing CI for feast-operator image builds test: Add missing CI for feast-operator image builds May 7, 2024
@tchughesiv tchughesiv changed the title test: Add missing CI for feast-operator image builds ci: Add missing CI for feast-operator image builds May 7, 2024
@tchughesiv
Copy link
Contributor Author

tchughesiv commented May 8, 2024

@woop is this something you can help with? the creation of those repos that is.
or, maybe increased perms for @franciscojavierarceo or @jeremyary so they can handle it?

@woop
Copy link
Member

woop commented May 13, 2024

Checking this out right now.

@woop
Copy link
Member

woop commented May 13, 2024

Just to confirm, is there a reason we'd use kf-feast on GCR instead of dockerhub? Also, @franciscojavierarceo can you confirm what access you have already?

@tchughesiv
Copy link
Contributor Author

tchughesiv commented May 13, 2024

@woop its used in .github/workflows/master_only.yml, which is named "integration-tests-and-build". it appears to run upon PR merge into the master branch and is a way a ensure image builds/pushes are working prior to release.

@franciscojavierarceo
Copy link
Member

I have access to Dockerhub but not gcr

@woop
Copy link
Member

woop commented May 19, 2024

Hey @franciscojavierarceo you should have GCR access now if you check your GCP console.

@tchughesiv tchughesiv force-pushed the feast-operator-ci-dev branch 3 times, most recently from 070c8d4 to 1719d65 Compare June 19, 2024 14:36
@tchughesiv
Copy link
Contributor Author

tchughesiv commented Jun 19, 2024

@franciscojavierarceo @woop were these repos ever added? should we revisit this? it may help us troubleshoot some of the release build issues we've been facing. don't forget to add write perms for the gh actions robot account.

@franciscojavierarceo
Copy link
Member

Sorry been out of the loop on this one @tchughesiv does this still need review or action from me?

@dmartinol dmartinol self-requested a review October 29, 2024 14:47
@franciscojavierarceo
Copy link
Member

@dmartinol looks like you need to fix some conflicts first

@tchughesiv tchughesiv force-pushed the feast-operator-ci-dev branch from 1719d65 to 894d280 Compare December 29, 2024 17:07
Signed-off-by: Tommy Hughes <tohughes@redhat.com>
@tchughesiv tchughesiv force-pushed the feast-operator-ci-dev branch from 6b84a1a to 1f3c60a Compare December 29, 2024 17:10
@tchughesiv
Copy link
Contributor Author

@franciscojavierarceo fixed...
but as a reminder, this PR shouldn't be merged until the repos in the description are created and provided push access to the workflow robot account.

@franciscojavierarceo
Copy link
Member

@tchughesiv so the robot should have access. The gcr.io/kf-feast artifact registry already exits so I believe once we try to push the images it should create the artifact/image.

@tchughesiv
Copy link
Contributor Author

@franciscojavierarceo i guess it's worth a shot, but my expectation is that those new repos will have to exist first. especially because this registry has been deprecated and there's a migration guide.
https://cloud.google.com/artifact-registry/docs/transition/auto-migrate-gcr-ar

Copy link
Member

@franciscojavierarceo franciscojavierarceo left a comment

Choose a reason for hiding this comment

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

let's give this a try

@franciscojavierarceo franciscojavierarceo merged commit e983882 into feast-dev:master Jan 9, 2025
20 of 21 checks passed
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.

5 participants