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

fix rock for ckf-1.7 #15

Merged
merged 1 commit into from
Dec 14, 2023
Merged

fix rock for ckf-1.7 #15

merged 1 commit into from
Dec 14, 2023

Conversation

ca-scribner
Copy link
Contributor

@ca-scribner ca-scribner commented Dec 7, 2023

previously, the rock's workload would not start, instead crashing on:

level=fatal msg="open web/templates/default: no such file or directory"

because the web files were not copied to the working directory. This commit refactors the rock and the working directory to be identical to the upstream docker image

Reviewer notes

  • integration tests are not set up yet for this repo. This PR should make it past the rock building steps of the test CI, but fail during rock testing

@ca-scribner ca-scribner requested a review from a team as a code owner December 7, 2023 20:53
previously, the rock's workload would not start, instead crashing on:

> level=fatal msg="open web/templates/default: no such file or directory"

because the web files were not copied to the working directory.  This commit refactors the rock and the working directory to be identical to the upstream docker image
@orfeas-k
Copy link
Contributor

orfeas-k commented Dec 8, 2023

Haven't reviewed the PR but would it make sense updating those too in this PR to be CKF 1.7 compatible?

@ca-scribner
Copy link
Contributor Author

@orfeas-k re microk8s and juju versions, that's a good question... that workflow doesn't capture how different tracks need different settings - I'm not sure how to capture that

either way, there is no actual integration test in this repo :D So that part of the testing wont matter atm anyway. Can we tackle this in #13?

@orfeas-k
Copy link
Contributor

that workflow doesn't capture how different tracks need different settings

That should be job of the repo's branches I guess and at least in one rock repos, that's more than fine. We can def tackle this as part of #13.

Copy link
Contributor

@orfeas-k orfeas-k left a comment

Choose a reason for hiding this comment

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

Tested the rock in a CKF 1.7 deployment and works. I created a notebook and connected to it as well. @kimwnasptd had some comments in a live discussion but they can be tackled in a future PR since this ROCK is functional. Thank you @ca-scribner

@NohaIhab NohaIhab merged commit 017f09f into track/ckf-1.7 Dec 14, 2023
2 of 3 checks passed
@kimwnasptd kimwnasptd deleted the KF-5091-fix-rock-for-1.7 branch December 14, 2023 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants