Skip to content

Commit 349ee7b

Browse files
committed
address some review comments
Signed-off-by: Lionel Villard <villard@us.ibm.com>
1 parent b1f1b4b commit 349ee7b

File tree

4 files changed

+20
-35
lines changed

4 files changed

+20
-35
lines changed

Makefile

Lines changed: 12 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,11 @@ NAMESPACE ?= hc4ai-operator
2222
VLLM_SIMULATOR_TAG ?= v0.5.0
2323
export VLLM_SIMULATOR_TAG
2424

25-
ACTIVATOR_IMAGE_TAG_BASE ?= $(IMAGE_REGISTRY)/$(PROJECT_NAME)-activator
26-
ACTIVATOR_IMG = $(ACTIVATOR_IMAGE_TAG_BASE):$(EPP_TAG)
25+
ACTIVATOR_IMAGE_NAME ?= llm-d-activator
26+
ACTIVATOR_NAME ?= activator
27+
ACTIVATOR_TAG ?= dev
28+
ACTIVATOR_IMAGE_TAG_BASE ?= $(IMAGE_REGISTRY)/$(ACTIVATOR_IMAGE_NAME)
29+
ACTIVATOR_IMG = $(ACTIVATOR_IMAGE_TAG_BASE):$(ACTIVATOR_TAG)
2730

2831
# Map go arch to typos arch
2932
ifeq ($(TARGETARCH),amd64)
@@ -62,10 +65,13 @@ SRC = $(shell find . -type f -name '*.go')
6265
# Internal variables for generic targets
6366
epp_IMAGE = $(IMG)
6467
sidecar_IMAGE = $(SIDECAR_IMG)
68+
activator_IMAGE = $(ACTIVATOR_IMG)
6569
epp_NAME = epp
6670
sidecar_NAME = $(SIDECAR_NAME)
71+
activator_NAME = $(ACTIVATOR_NAME)
6772
epp_LDFLAGS = -ldflags="$(LDFLAGS)"
6873
sidecar_LDFLAGS =
74+
activator_LDFLAGS = -ldflags="$(LDFLAGS)"
6975
epp_TEST_FILES = go list ./... | grep -v /test/ | grep -v ./pkg/sidecar/
7076
sidecar_TEST_FILES = go list ./pkg/sidecar/...
7177

@@ -138,25 +144,17 @@ lint: check-golangci-lint check-typos ## Run lint
138144
##@ Build
139145

140146
.PHONY: build
141-
build: build-epp build-sidecar ## Build the project
147+
build: build-epp build-sidecar build-activator ## Build the project
142148

143149
.PHONY: build-%
144150
build-%: check-go install-dependencies download-tokenizer ## Build the project
145151
@printf "\033[33;1m==== Building ====\033[0m\n"
146152
go build $($*_LDFLAGS) -o bin/$($*_NAME) cmd/$($*_NAME)/main.go
147153

148-
##@ Build Activator
149-
150-
.PHONY: activator-build
151-
activator-build: check-go install-dependencies download-tokenizer ## Build the project
152-
@printf "\033[33;1m==== Building ====\033[0m\n"
153-
go build -ldflags="$(LDFLAGS)" -o bin/activator cmd/activator/main.go
154-
155-
156154
##@ Container Build/Push
157155

158156
.PHONY: image-build
159-
image-build: image-build-epp image-build-sidecar ## Build Docker image
157+
image-build: image-build-epp image-build-sidecar image-build-activator ## Build Docker image
160158

161159
.PHONY: image-build-%
162160
image-build-%: check-container-tool ## Build Docker image ## Build Docker image using $(CONTAINER_RUNTIME)
@@ -170,7 +168,7 @@ image-build-%: check-container-tool ## Build Docker image ## Build Docker image
170168
-t $($*_IMAGE) -f Dockerfile.$* .
171169

172170
.PHONY: image-push
173-
image-push: image-push-epp image-push-sidecar ## Push container images to registry
171+
image-push: image-push-epp image-push-sidecar image-push-activator ## Push container images to registry
174172

175173
.PHONY: image-push-%
176174
image-push-%: check-container-tool ## Push container image to registry
@@ -182,22 +180,6 @@ image-pull: check-container-tool ## Pull all related images using $(CONTAINER_RU
182180
@printf "\033[33;1m==== Pulling Container images ====\033[0m\n"
183181
./scripts/pull_images.sh
184182

185-
.PHONY: activator-image-build
186-
activator-image-build: ## Build the activator image using Docker Buildx.
187-
$(CONTAINER_TOOL) build \
188-
--platform linux/$(TARGETARCH) \
189-
--build-arg TARGETOS=linux \
190-
--build-arg TARGETARCH=${TARGETARCH} \
191-
--build-arg COMMIT_SHA=${GIT_COMMIT_SHA} \
192-
--build-arg BUILD_REF=${BUILD_REF} \
193-
-t $(ACTIVATOR_IMG) \
194-
-f Dockerfile.activator .
195-
196-
.PHONY: activator-image-push
197-
activator-image-push: check-container-tool load-version-json ## Push Activator Docker image $(ACTIVATOR_IMG) to registry
198-
@printf "\033[33;1m==== Pushing Activator Docker image $(ACTIVATOR_IMG) ====\033[0m\n"
199-
$(CONTAINER_TOOL) push $(ACTIVATOR_IMG)
200-
201183
##@ Install/Uninstall Targets
202184

203185
# Default install/uninstall (Docker)
@@ -363,7 +345,7 @@ check-container-tool:
363345
else \
364346
echo "✅ Container tool '$(CONTAINER_RUNTIME)' found."; \
365347
fi
366-
348+
367349

368350
.PHONY: check-kubectl
369351
check-kubectl:

charts/activator/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ The following table list the configurable parameters of the chart.
3636
| `activator.image.pullPolicy` | Image pull policy for the container. Possible values: `Always`, `IfNotPresent`, or `Never`. Defaults to `Always`. |
3737
| `inferenceGateway.port` | The port of the Gateway. Defaults to `80`. |
3838
| `inferencePool.name` | The name of the InferencePool to target. |
39-
| `inferencePool.apiVersion` | The API version of the InferencePool. Defaults to `inference.networking.x-k8s.io`. |
39+
| `inferencePool.apiVersion` | The API version of the InferencePool. Defaults to `inference.networking.k8s.io`. |
4040
| `route.name` | The name of the HTTPRoute to attach the activator to. |
4141

4242
## Notes

charts/activator/values.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ route:
1313

1414
inferencePool:
1515
name: inference-pool
16-
group: inference.networking.x-k8s.io
16+
group: inference.networking.k8s.io
1717

1818
inferenceGateway:
1919
port: 80

pkg/activator/requestcontrol/activator.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/go-logr/logr"
1111
"k8s.io/apimachinery/pkg/api/meta"
1212
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
13+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1314
"k8s.io/apimachinery/pkg/runtime/schema"
1415
"k8s.io/apimachinery/pkg/util/wait"
1516
"k8s.io/client-go/dynamic"
@@ -179,14 +180,16 @@ func (a *Activator) inferencePoolPodsReady(logger logr.Logger, namespace, objnam
179180
}
180181

181182
// NOTE: this assumes that the target object has a status.readyReplicas field
182-
if readyReplicas, ok := unstructuredObj.Object["status"].(map[string]any)["readyReplicas"].(int64); ok {
183-
if numReplicas == int32(readyReplicas) {
183+
if readyReplicas, found, err := unstructured.NestedFieldNoCopy(unstructuredObj.Object, "status", "readyReplicas"); found {
184+
if numReplicas == int32(readyReplicas.(int64)) {
184185
logger.V(logutil.DEBUG).Info("Candidate pods are READY")
185186
return true, nil
186187
}
187188
logger.V(logutil.DEBUG).Info("Candidate pods are NOT READY")
189+
} else if err != nil {
190+
logger.Error(err, "Error getting readyReplicas - candidate pods for serving the request are NOT READY")
188191
} else {
189-
logger.V(logutil.DEBUG).Info("Object status.readyReplicas field is not set yet - candidate pods for serving the request are NOT READY ")
192+
logger.V(logutil.DEBUG).Info("Object status.readyReplicas field is not set yet - candidate pods for serving the request are NOT READY")
190193
return false, nil
191194
}
192195

0 commit comments

Comments
 (0)