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

Sync with main branch #266

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ vet: ## Run go vet against code.

.PHONY: test
test: manifests generate fmt vet envtest ## Run tests.
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" POD_NAMESPACE=default go test -v ./controllers/... -coverprofile cover.out
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" POD_NAMESPACE=default \
MESH_NAMESPACE=istio-system CONTROL_PLANE_NAME=istio-system go test -v ./controllers/... -coverprofile cover.out

.PHONY: e2e-test
e2e-test: manifests generate fmt vet ## Run e2e-tests.
Expand Down
1 change: 0 additions & 1 deletion config/runtimes/vllm-template.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ objects:
- "--port=8080"
- "--model=/mnt/models"
- "--served-model-name={{.Name}}"
- "--distributed-executor-backend=mp"
env:
- name: HF_HOME
value: /tmp/hf_home
Expand Down
2 changes: 0 additions & 2 deletions controllers/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ package constants
const (
InferenceServiceKind = "InferenceService"

IstioNamespace = "istio-system"
IstioControlPlaneName = "data-science-smcp"
ServiceMeshMemberRollName = "default"
ServiceMeshMemberName = "default"
IstioIngressService = "istio-ingressgateway"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,8 @@ func createAuthorizationPolicy(authPolicyFile string) error {

gvr := istiosec_v1b1.SchemeGroupVersion.WithResource("authorizationpolicies")
resource := dynamicClient.Resource(gvr)
_, createErr := resource.Namespace(constants.IstioNamespace).Create(context.TODO(), obj, metav1.CreateOptions{})
_, meshNamespace := utils.GetIstioControlPlaneName(ctx, cli)
_, createErr := resource.Namespace(meshNamespace).Create(context.TODO(), obj, metav1.CreateOptions{})

return createErr
}
Expand Down Expand Up @@ -371,5 +372,6 @@ func deleteAuthorizationPolicy(authPolicyFile string) error {
}

gvr := istiosec_v1b1.SchemeGroupVersion.WithResource("authorizationpolicies")
return dynamicClient.Resource(gvr).Namespace(constants.IstioNamespace).Delete(context.TODO(), obj.GetName(), metav1.DeleteOptions{})
_, meshNamespace := utils.GetIstioControlPlaneName(ctx, cli)
return dynamicClient.Resource(gvr).Namespace(meshNamespace).Delete(context.TODO(), obj.GetName(), metav1.DeleteOptions{})
}
6 changes: 4 additions & 2 deletions controllers/kserve_inferenceservice_controller_mesh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/opendatahub-io/odh-model-controller/controllers/constants"
"github.com/opendatahub-io/odh-model-controller/controllers/utils"
)

var _ = Describe("The KServe mesh reconciler", func() {
Expand Down Expand Up @@ -43,6 +44,7 @@ var _ = Describe("The KServe mesh reconciler", func() {
}

createUserOwnedMeshEnrolment := func(namespace string) *maistrav1.ServiceMeshMember {
controlPlaneName, meshNamespace := utils.GetIstioControlPlaneName(ctx, cli)
smm := &maistrav1.ServiceMeshMember{
ObjectMeta: metav1.ObjectMeta{
Name: constants.ServiceMeshMemberName,
Expand All @@ -52,8 +54,8 @@ var _ = Describe("The KServe mesh reconciler", func() {
},
Spec: maistrav1.ServiceMeshMemberSpec{
ControlPlaneRef: maistrav1.ServiceMeshControlPlaneRef{
Name: constants.IstioControlPlaneName,
Namespace: constants.IstioNamespace,
Name: controlPlaneName,
Namespace: meshNamespace,
}},
Status: maistrav1.ServiceMeshMemberStatus{},
}
Expand Down
23 changes: 22 additions & 1 deletion controllers/kserve_inferenceservice_controller_metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ const (
KserveOvmsInferenceServiceName = "example-onnx-mnist"
UnsupportedMetricsInferenceServiceName = "sklearn-v2-iris"
NilRuntimeInferenceServiceName = "sklearn-v2-iris-no-runtime"
NilModelInferenceServiceName = "custom-runtime"
UnsupportedMetricsInferenceServicePath = "./testdata/deploy/kserve-unsupported-metrics-inference-service.yaml"
UnsupprtedMetricsServingRuntimePath = "./testdata/deploy/kserve-unsupported-metrics-serving-runtime.yaml"
NilRuntimeInferenceServicePath = "./testdata/deploy/kserve-nil-runtime-inference-service.yaml"
NilModelInferenceServicePath = "./testdata/deploy/kserve-nil-model-inference-service.yaml"
)

var _ = Describe("The KServe Dashboard reconciler", func() {
Expand Down Expand Up @@ -126,6 +128,25 @@ var _ = Describe("The KServe Dashboard reconciler", func() {
}
Expect(compareConfigMap(metricsConfigMap, expectedmetricsConfigMap)).Should(BeTrue())
})

It("if the isvc does not have the model field specified, an unsupported metrics configmap should be created", func() {
_ = createInferenceService(testNs, NilModelInferenceServiceName, NilModelInferenceServicePath)

metricsConfigMap, err := waitForConfigMap(cli, testNs, NilModelInferenceServiceName+constants.KserveMetricsConfigMapNameSuffix, 30, 1*time.Second)
Expect(err).NotTo(HaveOccurred())
Expect(metricsConfigMap).NotTo(BeNil())

expectedCM := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: NilModelInferenceServiceName + constants.KserveMetricsConfigMapNameSuffix,
Namespace: testNs,
},
Data: map[string]string{
"supported": "false",
},
}
Expect(compareConfigMap(metricsConfigMap, expectedCM)).Should(BeTrue())
})
})

When("deleting the deployed models", func() {
Expand All @@ -147,7 +168,7 @@ var _ = Describe("The KServe Dashboard reconciler", func() {
Expect(cli.Delete(ctx, SklearnInferenceService)).Should(Succeed())
Eventually(func() error {
configmap := &corev1.ConfigMap{}
key := types.NamespacedName{Name: KserveOvmsInferenceServiceName + constants.KserveMetricsConfigMapNameSuffix, Namespace: SklearnInferenceService.Namespace}
key := types.NamespacedName{Name: UnsupportedMetricsInferenceServiceName + constants.KserveMetricsConfigMapNameSuffix, Namespace: SklearnInferenceService.Namespace}
err := cli.Get(ctx, key, configmap)
return err
}, timeout, interval).ShouldNot(Succeed())
Expand Down
26 changes: 17 additions & 9 deletions controllers/kserve_inferenceservice_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
. "github.com/onsi/gomega/gstruct"
gomegatypes "github.com/onsi/gomega/types"
"github.com/opendatahub-io/odh-model-controller/controllers/constants"
"github.com/opendatahub-io/odh-model-controller/controllers/utils"
routev1 "github.com/openshift/api/route/v1"
istioclientv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -69,6 +70,7 @@ var _ = Describe("The Openshift Kserve model controller", func() {
})

It("With Kserve InferenceService a Route be created", func() {
_, meshNamespace := utils.GetIstioControlPlaneName(ctx, cli)
inferenceService := &kservev1beta1.InferenceService{}
err := convertToStructuredResource(KserveInferenceServicePath1, inferenceService)
Expect(err).NotTo(HaveOccurred())
Expand All @@ -79,7 +81,7 @@ var _ = Describe("The Openshift Kserve model controller", func() {
By("By checking that the controller has not created the Route")
Consistently(func() error {
route := &routev1.Route{}
key := types.NamespacedName{Name: getKServeRouteName(inferenceService), Namespace: constants.IstioNamespace}
key := types.NamespacedName{Name: getKServeRouteName(inferenceService), Namespace: meshNamespace}
err = cli.Get(ctx, key, route)
return err
}, timeout, interval).Should(HaveOccurred())
Expand All @@ -98,7 +100,7 @@ var _ = Describe("The Openshift Kserve model controller", func() {
By("By checking that the controller has created the Route")
Eventually(func() error {
route := &routev1.Route{}
key := types.NamespacedName{Name: getKServeRouteName(inferenceService), Namespace: constants.IstioNamespace}
key := types.NamespacedName{Name: getKServeRouteName(inferenceService), Namespace: meshNamespace}
err = cli.Get(ctx, key, route)
return err
}, timeout, interval).Should(Succeed())
Expand Down Expand Up @@ -192,16 +194,18 @@ var _ = Describe("The Openshift Kserve model controller", func() {
err = cli.Status().Update(ctx, deployedInferenceService)
Expect(err).NotTo(HaveOccurred())

_, meshNamespace := utils.GetIstioControlPlaneName(ctx, cli)

// Verify that the certificate secret is created in the istio-system namespace.
Eventually(func() error {
secret := &corev1.Secret{}
return cli.Get(ctx, client.ObjectKey{Namespace: constants.IstioNamespace, Name: fmt.Sprintf("%s-%s", inferenceService.Name, inferenceService.Namespace)}, secret)
return cli.Get(ctx, client.ObjectKey{Namespace: meshNamespace, Name: fmt.Sprintf("%s-%s", inferenceService.Name, inferenceService.Namespace)}, secret)
}, timeout, interval).Should(Succeed())

// Verify that the gateway is updated in the istio-system namespace.
var gateway *istioclientv1beta1.Gateway
Eventually(func() error {
gateway, err = waitForUpdatedGatewayCompletion(cli, "add", constants.IstioNamespace, constants.KServeGatewayName, inferenceService.Name)
gateway, err = waitForUpdatedGatewayCompletion(cli, "add", meshNamespace, constants.KServeGatewayName, inferenceService.Name)
return err
}, timeout, interval).Should(Succeed())

Expand Down Expand Up @@ -246,6 +250,7 @@ var _ = Describe("The Openshift Kserve model controller", func() {
ctx := context.Background()
testNamespace := Namespaces.Create(cli)
testNs = testNamespace.Name
_, meshNamespace := utils.GetIstioControlPlaneName(ctx, cli)

inferenceServiceConfig := &corev1.ConfigMap{}
Expect(convertToStructuredResource(InferenceServiceConfigPath1, inferenceServiceConfig)).To(Succeed())
Expand Down Expand Up @@ -308,13 +313,13 @@ var _ = Describe("The Openshift Kserve model controller", func() {
}, timeout, interval).Should(Succeed())

Eventually(func() error {
return cli.Get(ctx, client.ObjectKey{Namespace: constants.IstioNamespace, Name: fmt.Sprintf("%s-%s", inferenceService.Name, inferenceService.Namespace)}, secret)
return cli.Get(ctx, client.ObjectKey{Namespace: meshNamespace, Name: fmt.Sprintf("%s-%s", inferenceService.Name, inferenceService.Namespace)}, secret)
}, timeout, interval).Should(Succeed())

// Verify that the gateway is updated in the istio-system namespace.
var gateway *istioclientv1beta1.Gateway
Eventually(func() error {
gateway, err = waitForUpdatedGatewayCompletion(cli, "add", constants.IstioNamespace, constants.KServeGatewayName, inferenceService.Name)
gateway, err = waitForUpdatedGatewayCompletion(cli, "add", meshNamespace, constants.KServeGatewayName, inferenceService.Name)
return err
}, timeout, interval).Should(Succeed())

Expand Down Expand Up @@ -345,9 +350,10 @@ var _ = Describe("The Openshift Kserve model controller", func() {
Expect(err).NotTo(HaveOccurred())

// Verify that the certificate secret in the istio-system namespace is updated.
_, meshNamespace := utils.GetIstioControlPlaneName(ctx, cli)
destSecret := &corev1.Secret{}
Eventually(func() error {
Expect(cli.Get(ctx, client.ObjectKey{Namespace: constants.IstioNamespace, Name: fmt.Sprintf("%s-%s", deployedInferenceService.Name, deployedInferenceService.Namespace)}, destSecret)).Should(Succeed())
Expect(cli.Get(ctx, client.ObjectKey{Namespace: meshNamespace, Name: fmt.Sprintf("%s-%s", deployedInferenceService.Name, deployedInferenceService.Namespace)}, destSecret)).Should(Succeed())
if string(destSecret.Data["tls.crt"]) != updatedDataString {
return fmt.Errorf("destSecret is not updated yet")
}
Expand All @@ -366,10 +372,12 @@ var _ = Describe("The Openshift Kserve model controller", func() {
Expect(err).NotTo(HaveOccurred())
Expect(cli.Delete(ctx, deployedInferenceService)).Should(Succeed())

_, meshNamespace := utils.GetIstioControlPlaneName(ctx, cli)

// Verify that the gateway is updated in the istio-system namespace.
var gateway *istioclientv1beta1.Gateway
Eventually(func() error {
gateway, err = waitForUpdatedGatewayCompletion(cli, "delete", constants.IstioNamespace, constants.KServeGatewayName, isvcName)
gateway, err = waitForUpdatedGatewayCompletion(cli, "delete", meshNamespace, constants.KServeGatewayName, isvcName)
return err
}, timeout, interval).Should(Succeed())

Expand All @@ -380,7 +388,7 @@ var _ = Describe("The Openshift Kserve model controller", func() {
// Ensure that the synced Secret is successfully deleted within the istio-system namespace.
secret := &corev1.Secret{}
Eventually(func() error {
return cli.Get(ctx, client.ObjectKey{Namespace: constants.IstioNamespace, Name: fmt.Sprintf("%s-%s", isvcName, constants.IstioNamespace)}, secret)
return cli.Get(ctx, client.ObjectKey{Namespace: meshNamespace, Name: fmt.Sprintf("%s-%s", isvcName, meshNamespace)}, secret)
}, timeout, interval).ShouldNot(Succeed())
})
})
Expand Down
3 changes: 2 additions & 1 deletion controllers/reconcilers/kserve_authconfig_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package reconcilers
import (
"context"
"fmt"

"github.com/opendatahub-io/odh-model-controller/controllers/utils"

"github.com/go-logr/logr"
Expand Down Expand Up @@ -57,7 +58,7 @@ func NewKserveAuthConfigReconciler(client client.Client) *KserveAuthConfigReconc

func (r *KserveAuthConfigReconciler) Reconcile(ctx context.Context, log logr.Logger, isvc *kservev1beta1.InferenceService) error {
log.V(1).Info("Reconciling Authorino AuthConfig for InferenceService")
authorinoEnabled, capabilityErr := utils.VerifyIfMeshAuthorizationIsEnabled(context.Background(), r.client)
authorinoEnabled, capabilityErr := utils.VerifyIfMeshAuthorizationIsEnabled(ctx, r.client)
if capabilityErr != nil {
log.V(1).Error(capabilityErr, "Error while verifying if Authorino is enabled")
return nil
Expand Down
17 changes: 12 additions & 5 deletions controllers/reconcilers/kserve_metrics_dashboard_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,17 +95,24 @@ func (r *KserveMetricsDashboardReconciler) createDesiredResource(ctx context.Con
var servingRuntime string
runtime := &kservev1alpha1.ServingRuntime{}
supported := false

// there is the possibility to also have and nil model field, e.g:
//predictor:
// containers:
// - name: kserve-container
// image: user/custom-model:v1
if nil == isvc.Spec.Predictor.Model {
log.V(1).Info("no `predictor.model` field found in InferenceService, no metrics will be available")
return r.createConfigMap(isvc, false, log)
}

// resolve SR
isvcRuntime := isvc.Spec.Predictor.Model.Runtime
if isvcRuntime == nil {
runtime, err = utils.FindSupportingRuntimeForISvc(ctx, r.client, log, isvc)
if err != nil {
if errwrap.Contains(err, constants.NoSuitableRuntimeError) {
configmap, err := r.createConfigMap(isvc, false, log)
if err != nil {
return nil, err
}
return configmap, nil
return r.createConfigMap(isvc, false, log)
}
return nil, err
}
Expand Down
11 changes: 8 additions & 3 deletions controllers/reconcilers/kserve_route_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
constants2 "github.com/opendatahub-io/odh-model-controller/controllers/constants"
"github.com/opendatahub-io/odh-model-controller/controllers/processors"
"github.com/opendatahub-io/odh-model-controller/controllers/resources"
utils2 "github.com/opendatahub-io/odh-model-controller/controllers/utils"
v1 "github.com/openshift/api/route/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -75,7 +76,8 @@ func (r *KserveRouteReconciler) Reconcile(ctx context.Context, log logr.Logger,

func (r *KserveRouteReconciler) Delete(ctx context.Context, log logr.Logger, isvc *kservev1beta1.InferenceService) error {
log.V(1).Info("Deleting Kserve inference service generic route")
return r.routeHandler.DeleteRoute(ctx, types.NamespacedName{Name: getKServeRouteName(isvc), Namespace: constants2.IstioNamespace})
_, meshNamespace := utils2.GetIstioControlPlaneName(ctx, r.client)
return r.routeHandler.DeleteRoute(ctx, types.NamespacedName{Name: getKServeRouteName(isvc), Namespace: meshNamespace})
}

func (r *KserveRouteReconciler) Cleanup(_ context.Context, _ logr.Logger, _ string) error {
Expand Down Expand Up @@ -129,10 +131,12 @@ func (r *KserveRouteReconciler) createDesiredResource(isvc *kservev1beta1.Infere
}
}

_, meshNamespace := utils2.GetIstioControlPlaneName(context.Background(), r.client)

route := &v1.Route{
ObjectMeta: metav1.ObjectMeta{
Name: getKServeRouteName(isvc),
Namespace: constants2.IstioNamespace,
Namespace: meshNamespace,
Annotations: annotations,
Labels: isvc.Labels,
},
Expand All @@ -159,7 +163,8 @@ func (r *KserveRouteReconciler) createDesiredResource(isvc *kservev1beta1.Infere
}

func (r *KserveRouteReconciler) getExistingResource(ctx context.Context, log logr.Logger, isvc *kservev1beta1.InferenceService) (*v1.Route, error) {
return r.routeHandler.FetchRoute(ctx, log, types.NamespacedName{Name: getKServeRouteName(isvc), Namespace: constants2.IstioNamespace})
_, meshNamespace := utils2.GetIstioControlPlaneName(ctx, r.client)
return r.routeHandler.FetchRoute(ctx, log, types.NamespacedName{Name: getKServeRouteName(isvc), Namespace: meshNamespace})
}

func (r *KserveRouteReconciler) processDelta(ctx context.Context, log logr.Logger, desiredRoute *v1.Route, existingRoute *v1.Route) (err error) {
Expand Down
9 changes: 8 additions & 1 deletion controllers/storageconfig_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,14 @@ func (r *StorageSecretReconciler) reconcileSecret(secret *corev1.Secret,
Namespace: secret.Namespace,
}, odhGlobalCertConfigMap)

if err == nil {
if err != nil {
if apierrs.IsNotFound(err) {
log.Info("unable to fetch the ODH Global Cert ConfigMap", "error", err)

} else {
return err
}
} else {
odhCustomCertData = odhGlobalCertConfigMap.Data[constants.ODHCustomCACertFileName]
}

Expand Down
34 changes: 34 additions & 0 deletions controllers/storageconfig_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
const (
dataconnectionStringPath = "./testdata/secrets/dataconnection-string.yaml"
storageconfigEncodedPath = "./testdata/secrets/storageconfig-encoded.yaml"
storageconfigCertString = "./testdata/secrets/storageconfig-cert-string.yaml"
storageconfigEncodedUnmanagedPath = "./testdata/secrets/storageconfig-encoded-unmanaged.yaml"
storageconfigCertEncodedPath = "./testdata/secrets/storageconfig-cert-encoded.yaml"
storageconfigUpdatedCertEncodedPath = "./testdata/secrets/storageconfig-updated-cert-encoded.yaml"
Expand Down Expand Up @@ -183,6 +184,39 @@ var _ = Describe("StorageConfig controller", func() {
Expect(compareSecrets(updatedStorageconfigSecret, expectedUpdatedStorageConfigSecret)).Should((BeTrue()))
})
})

Context("when a configmap odh-trusted-ca-bundle does not exists", func() {
It("should not return error", func() {
dataconnectionStringSecret := &corev1.Secret{}

By("creating data connection secret")
err := convertToStructuredResource(dataconnectionStringPath, dataconnectionStringSecret)
Expect(err).NotTo(HaveOccurred())
Expect(cli.Create(ctx, dataconnectionStringSecret)).Should(Succeed())

storageconfigSecret, err := waitForSecret(cli, WorkingNamespace, constants.DefaultStorageConfig, 30, 1*time.Second)
Expect(err).NotTo(HaveOccurred())

// Check storage-config secret
expectedStorageConfigSecret := &corev1.Secret{}
err = convertToStructuredResource(storageconfigEncodedPath, expectedStorageConfigSecret)
Expect(err).NotTo(HaveOccurred())

stringMap := make(map[string]string)
for key, value := range storageconfigSecret.Data {
stringValue := string(value)
stringMap[key] = stringValue
}
fmt.Printf("storageconfigSecret %s\n", stringMap)
stringMap1 := make(map[string]string)
for key, value := range expectedStorageConfigSecret.Data {
stringValue := string(value)
stringMap1[key] = stringValue
}
Expect(compareSecrets(storageconfigSecret, expectedStorageConfigSecret)).Should((BeTrue()))
})
})

})

func updateSecretData(cli client.Client, namespace, secretName string, dataKey string, dataValue string) error {
Expand Down
Loading
Loading