-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add metacontroller for kubeflow 1.4 #1
Conversation
* install hook has MVP functionality. It installs correctly and monitors the stateful until it is ready. Does not monitor/check other objs * includes basic test coverage Todo: * update-status hook * remove hook * all TODO items in code
Hook fails because kubectl cannot access environment. Maybe juju sets up the remove hook env differently than install?
Current version will restore the state if it notices the statefulset is missing/incomplete
This charm is missing RBAC needed for the status checking. Currently it breaks when RBAC is enabled |
Uses juju/juju#13442 (landed in juju 2.9.18)
WIP. Missing unit tests and not fully tested
Add tests for catch of trust-related error
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ca-scribner, few comments.
You can stop the behaviour where @patch("lightkube.core.client.GenericSyncClient", Mock) |
client fails when kube config cannot be found on system (such as in GH runner)
Install now attempts to create all resources, failing over to patching any that already exist (and logging this to debug-log). This is not foolproof (it could overwrite existing resources) Also changed: * k8s resource names now slightly more unique, attempting to avoid conflicts * changed CI name for `test_build_and_deploy` to `test_build_and_deploy_with_trust` to make it easier to run just that test during debugging
Seems to be colliding with the regular deploy test, maybe because the second test starts before the first is fully torn down? Not sure, but both work in isolation locally.
* refactor _create and _render to minimize duplicated code * misc minor changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Includes: * Removing properties for simplicity * Using tenacity.retry instead of while loop for checking resources * Using CheckFailed exception for raising errors in helpers * Scheduled weekly integration CI
Restore assignment of mock object that was removed by mistake
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This is an MVP of the charm. A fully implemented charm should have better status handling, teardown, etc.PR is now fully fleshed out charm for the Metacontroller. If things are missing, I missed them!