-
Notifications
You must be signed in to change notification settings - Fork 6
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
refactor: apply a workload Service instead of using juju created one #173
refactor: apply a workload Service instead of using juju created one #173
Conversation
To avoid inconsistent behaviours, it is preferrable to apply and use a Service owned by the charm so it can be rendered as needed by the controller.
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.
hi @DnPlas
first, left a small comment
second, can you explain what was the issue you were seeing? what is the problem this PR is trying to solve? I'm struggling to interpret that. Would be better if it is documented in an issue so we have a reference, wdyt?
The problem is that the Service created by juju will try to be used by two different Pods (the training operator charm and the training operator workload). This will cause inconsistencies like when we try to reach the metrics endpoint. We know that the training operator workload has it, but the charm doesn't, so sometimes trying to reach this endpoint will result in:
To avoid having both Pods "fighting" over the same Service (which btw is not at all recommended, it goes against the logic of services), I'm proposing that the workload has its own. |
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.
thx @DnPlas, tested and I was able to curl the metrics endpoint successfully.
Small nit for a typo then I'm approving, feel free to ping me to approve.
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
…173) * refactor: apply a workload Service instead of using juju created one To avoid inconsistent behaviours, it is preferrable to apply and use a Service owned by the charm so it can be rendered as needed by the controller.
…` for workload, also bumps training-operator 1.7->1.8 (#167) * pin integration test dependencies, refactor constants in tests (#164) * refactor: deploy the training-operator with kubernetes resources (#161) * chore: bump training-operator v1.7 -> v1.8 (#162) * refactor: apply a workload Service instead of using juju created one (#173) * tests: skip test_upgrade due to #170 (#171) * build, tests: bump charmed-kubeflow-chisme 0.4.0 -> 0.4.1 (#172) Fixes #159
To avoid inconsistent behaviours, it is preferrable to apply and use a Service owned by the charm so it can be rendered as needed by the controller.