Allow ModelMesh and KServe to run in same namespace. #132
Conversation
Signed-off-by: Vaibhav Jain <[email protected]>
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: vaibhavjainwiz The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
vaibhavjainwiz
requested review from
spolti,
Jooho and
VedantMahabaleshwarkar
Signed-off-by: Vaibhav Jain <[email protected]>
| Watches(&source.Kind{Type: &networkingv1.NetworkPolicy{}}, | ||
| handler.EnqueueRequestsFromMapFunc(func(o client.Object) []reconcile.Request { | ||
| r.log.Info("Reconcile event triggered by Network Policy: " + o.GetName()) | ||
| return r.getReconcileRequestsOnUpdateOfServingRuntime(o) |
There was a problem hiding this comment.
Is it right to call the getReconcileRequestsOnUpdateOfServingRuntime and not the getReconcileRequestsOnUpdateOfNetworkPolicy function?
There was a problem hiding this comment.
yes, you are right. I make the wrong function call. Its not tracked in my testing earlier because both function do similiar task.
I had fixed the function call now.
| return reconcileRequests | ||
| } | ||
|
|
||
| func (r *OpenshiftInferenceServiceReconciler) getReconcileRequestsOnUpdateOfNetworkPolicy(o client.Object) []reconcile.Request { |
There was a problem hiding this comment.
Do we really need this function? It seems to be equal, except 1 comment to the getReconcileRequestsOnUpdateOfServingRuntime
There was a problem hiding this comment.
for now they are doing same task to return list of all InferenceServices in that namespace but we have task in our backlog to update the logic of getReconcileRequestsOnUpdateOfServingRuntime function.
https://issues.redhat.com/browse/RHOAIENG-1265
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
| ) | ||
|
|
||
| type MMControllerNetworkPolicyReconciler struct { |
There was a problem hiding this comment.
Do we need the *Controller* into the name?
There was a problem hiding this comment.
To fix this issue, I need to implement two NetworkPolicies :
- allow access to expose Route
- allow calls from odh-model-controller
To make visible distinction between these two NetworkPolicies, I had create file with below names :
- mm_route_networkpolicy_reconciler.go
- mm_controller_networkpolicy_reconciler.go
I am open to change their name to be more meaningful.
Signed-off-by: Vaibhav Jain <[email protected]>
|
Setting this PR to do-not-merge because NetworkPolicies created with this PR would not be required any more after https://issues.redhat.com/browse/RHOAIENG-1003 fix. |
May this PR is still relevant? I think RHOAIENG-1003 was reverted. |
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
Do we still need this PR? |
|
@vaibhavjainwiz: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Closing as stale... |
Solving following issue with this PR:
Description
How Has This Been Tested?
Setup Env
Please follow below instruction to setup your env:
https://gist.github.com/vaibhavjainwiz/61045727dfda727f7577a6f97ca46563
Validate
Merge criteria: