Skip to content

Commit 48c6273

Browse files
committed
apply review changes from copilot
Signed-off-by: Mohammed Abdi <mohammed.munir.abdi@ibm.com>
1 parent 533fd7d commit 48c6273

4 files changed

Lines changed: 81 additions & 11 deletions

File tree

Makefile

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,10 +299,12 @@ lint: golangci-lint ## Run golangci-lint linter
299299
$(GOLANGCI_LINT) run
300300

301301
.PHONY: lint-deploy-scripts
302-
lint-deploy-scripts: ## Run bash -n for deploy/install.sh and deploy/lib/*.sh
302+
lint-deploy-scripts: ## Run bash -n for deploy/install.sh, deploy/lib/*.sh, and deploy plugins
303303
@echo "Syntax-checking deploy shell scripts..."
304304
@bash -n deploy/install.sh
305305
@for script in deploy/lib/*.sh; do bash -n "$$script"; done
306+
@for script in deploy/*/install.sh; do if [ -f "$$script" ]; then bash -n "$$script"; fi; done
307+
@for script in deploy/kind-emulator/*.sh; do if [ -f "$$script" ]; then bash -n "$$script"; fi; done
306308
@echo "deploy script syntax OK"
307309

308310
.PHONY: smoke-deploy-scripts

deploy/lib/cli.sh

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,11 @@ parse_args() {
102102
fi
103103
;;
104104
-h|--help) print_help; exit 0 ;;
105-
*) log_error "Unknown option: $1"; print_help; exit 1 ;;
105+
*)
106+
echo "Error: Unknown option: $1" >&2
107+
print_help
108+
exit 1
109+
;;
106110
esac
107111
done
108112
}

deploy/lib/install_core.sh

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ main() {
5454
set_tls_verification
5555
set_wva_logging_level
5656

57-
if [[ "$CLUSTER_TYPE" == "kind" ]]; then
57+
if [[ "${CLUSTER_TYPE:-}" == "kind" ]]; then
5858
log_info "Kind cluster detected - setting environment to kind-emulated"
5959
ENVIRONMENT="kind-emulator"
6060
fi
@@ -65,9 +65,8 @@ main() {
6565
source "$SCRIPT_DIR/$ENVIRONMENT/install.sh"
6666

6767
# Run environment-specific prerequisite checks if function exists
68-
if declare -f check_prerequisites > /dev/null; then
68+
if declare -f check_specific_prerequisites > /dev/null; then
6969
if [ "$SKIP_CHECKS" != "true" ]; then
70-
check_prerequisites
7170
check_specific_prerequisites
7271
fi
7372
fi

test/e2e/fixtures/inference_objective.go

Lines changed: 71 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ package fixtures
33
import (
44
"context"
55
"fmt"
6+
"os"
7+
"reflect"
8+
"strings"
69

710
apierrors "k8s.io/apimachinery/pkg/api/errors"
811
"k8s.io/apimachinery/pkg/api/meta"
@@ -25,11 +28,38 @@ var inferenceObjectiveGVR = schema.GroupVersionResource{
2528
// Returns applied=true if the object exists or was created. If the InferenceObjective API is not
2629
// available on the cluster, returns (false, nil).
2730
func EnsureInferenceObjective(ctx context.Context, dc dynamic.Interface, namespace, poolName string) (applied bool, err error) {
28-
obj := buildInferenceObjective(namespace, poolName)
2931
ri := dc.Resource(inferenceObjectiveGVR).Namespace(namespace)
32+
poolGroup, gErr := resolveInferencePoolGroup(ctx, dc, namespace, poolName)
33+
if gErr != nil {
34+
return false, gErr
35+
}
36+
obj := buildInferenceObjective(namespace, poolName, poolGroup)
3037

3138
if _, cErr := ri.Create(ctx, obj, metav1.CreateOptions{}); cErr != nil {
3239
if apierrors.IsAlreadyExists(cErr) {
40+
current, getErr := ri.Get(ctx, "e2e-default", metav1.GetOptions{})
41+
if getErr != nil {
42+
return false, fmt.Errorf("get existing InferenceObjective e2e-default: %w", getErr)
43+
}
44+
45+
currentSpec, _, specErr := unstructured.NestedMap(current.Object, "spec")
46+
if specErr != nil {
47+
return false, fmt.Errorf("read existing InferenceObjective spec: %w", specErr)
48+
}
49+
desiredSpec, _, desiredErr := unstructured.NestedMap(obj.Object, "spec")
50+
if desiredErr != nil {
51+
return false, fmt.Errorf("read desired InferenceObjective spec: %w", desiredErr)
52+
}
53+
if reflect.DeepEqual(currentSpec, desiredSpec) {
54+
return true, nil
55+
}
56+
57+
if setErr := unstructured.SetNestedMap(current.Object, desiredSpec, "spec"); setErr != nil {
58+
return false, fmt.Errorf("set desired InferenceObjective spec: %w", setErr)
59+
}
60+
if _, uErr := ri.Update(ctx, current, metav1.UpdateOptions{}); uErr != nil {
61+
return false, fmt.Errorf("update InferenceObjective e2e-default: %w", uErr)
62+
}
3363
return true, nil
3464
}
3565
if inferenceObjectiveAPIMissing(cErr) {
@@ -49,7 +79,7 @@ func DeleteInferenceObjective(ctx context.Context, dc dynamic.Interface, namespa
4979
return err
5080
}
5181

52-
func buildInferenceObjective(namespace, poolName string) *unstructured.Unstructured {
82+
func buildInferenceObjective(namespace, poolName, poolGroup string) *unstructured.Unstructured {
5383
return &unstructured.Unstructured{
5484
Object: map[string]interface{}{
5585
"apiVersion": "inference.networking.x-k8s.io/v1alpha2",
@@ -63,7 +93,7 @@ func buildInferenceObjective(namespace, poolName string) *unstructured.Unstructu
6393
"poolRef": map[string]interface{}{
6494
"name": poolName,
6595
"kind": "InferencePool",
66-
"group": "inference.networking.k8s.io",
96+
"group": poolGroup,
6797
},
6898
},
6999
},
@@ -77,9 +107,44 @@ func inferenceObjectiveAPIMissing(err error) bool {
77107
if meta.IsNoMatchError(err) {
78108
return true
79109
}
80-
if apierrors.IsNotFound(err) {
110+
se, ok := err.(*apierrors.StatusError)
111+
if !ok {
112+
return false
113+
}
114+
if se.ErrStatus.Reason != metav1.StatusReasonNotFound {
115+
return false
116+
}
117+
if strings.Contains(strings.ToLower(se.ErrStatus.Message), "the server could not find the requested resource") {
81118
return true
82119
}
83-
// Discovery / dynamic client sometimes returns ResourceNotFound with Reason NotFound
84-
return apierrors.ReasonForError(err) == metav1.StatusReasonNotFound
120+
details := se.ErrStatus.Details
121+
if details == nil {
122+
return false
123+
}
124+
return details.Group == inferenceObjectiveGVR.Group && details.Kind == inferenceObjectiveGVR.Resource
125+
}
126+
127+
func resolveInferencePoolGroup(ctx context.Context, dc dynamic.Interface, namespace, poolName string) (string, error) {
128+
if envPoolGroup := os.Getenv("POOL_GROUP"); envPoolGroup != "" {
129+
return envPoolGroup, nil
130+
}
131+
132+
inferencePoolCandidates := []schema.GroupVersionResource{
133+
{Group: "inference.networking.k8s.io", Version: "v1", Resource: "inferencepools"},
134+
{Group: "inference.networking.x-k8s.io", Version: "v1alpha2", Resource: "inferencepools"},
135+
}
136+
137+
for _, gvr := range inferencePoolCandidates {
138+
_, err := dc.Resource(gvr).Namespace(namespace).Get(ctx, poolName, metav1.GetOptions{})
139+
if err == nil {
140+
return gvr.Group, nil
141+
}
142+
if apierrors.IsNotFound(err) || meta.IsNoMatchError(err) {
143+
continue
144+
}
145+
return "", fmt.Errorf("detect InferencePool API group for %q: %w", poolName, err)
146+
}
147+
148+
// Default to the primary group used by llm-d charts.
149+
return "inference.networking.k8s.io", nil
85150
}

0 commit comments

Comments
 (0)