Skip to content

Commit 2293410

Browse files
refactor(test): build-tag OCP scheme registrations and drop legacy SetupEnvTest (opendatahub-io#1329)
* refactor: guard OCP scheme registrations with distro build tags Prometheus Operator monitoring APIs, OpenShift Route, and Istio v1 scheme registrations were wired unconditionally, pulling OCP-only dependencies into upstream builds. This makes the upstream binary carry dead scheme registrations and pulls in imports that don't belong there. Uses the established companion file pattern (//go:build distro / !distro) to gate these registrations, keeping the upstream build surface clean while preserving full functionality in distro builds. Signed-off-by: Bartosz Majsak <bartosz.majsak@gmail.com> * refactor(test): migrate credentials suite to NewEnvTest, drop SetupEnvTest The credentials suite was the last caller of the legacy SetupEnvTest path. Migrate it to NewEnvTest().BuildEnvironment(), which uses a fresh per-suite scheme (t.Scheme) instead of mutating the global scheme.Scheme. With no callers left, remove SetupEnvTest and its build-tag companion files (envtest_setup_default.go / envtest_setup_ocp.go), collapsing the two test setup paths into one. Signed-off-by: Bartosz Majsak <bartosz.majsak@gmail.com> * simplify: drop manual capacity hints in scheme slice construction Replace make+append pairs with the append([]T{...}, rest...) idiom. The magic number capacity hints (5+len(distroSchemes), 8+len(...), etc.) silently drift whenever entries are added or removed. The composite literal approach allocates exactly once with no number to maintain. Signed-off-by: Bartosz Majsak <bartosz.majsak@gmail.com> * fix: wire GOTAGS through Dockerfile and docker-build for main controller routev1.AddToScheme was moved behind //go:build distro in main_schemes_ocp.go but the main Dockerfile had no ARG GOTAGS and the docker-build Makefile target was not passing --build-arg GOTAGS. This meant the binary was always compiled without the distro tag, main_schemes_default.go (no-op) was linked, and v1.Route was never registered - causing the InferenceGraph controller to crash on startup. Follow the pattern already in place for llmisvc-controller.Dockerfile and docker-build-llmisvc. Signed-off-by: Bartosz Majsak <bartosz.majsak@gmail.com> * docs(rules): extend build-tags rule to cover Dockerfiles and Makefile targets Adds rules 7 and 8 to catch the class of bug where a Dockerfile builds a Go binary that uses //go:build distro code but does not declare ARG GOTAGS or pass -tags to go build - and the corresponding Makefile target does not pass --build-arg GOTAGS. The canonical references are llmisvc-controller.Dockerfile (correct) and the main Dockerfile before this fix (incorrect). Signed-off-by: Bartosz Majsak <bartosz.majsak@gmail.com> --------- Signed-off-by: Bartosz Majsak <bartosz.majsak@gmail.com>
1 parent c4726c3 commit 2293410

File tree

14 files changed

+197
-75
lines changed

14 files changed

+197
-75
lines changed

.rules/build-tags.md

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,38 @@ lives in `*_ocp.go` files compiled only with `//go:build distro`; the upstream f
3636
full repository tree (diff-only mode), flag tentatively and ask the author to confirm whether a
3737
`*_default.go` companion exists in the same package.
3838

39+
## Build system propagation rules
40+
41+
These rules apply when the diff touches **Dockerfiles**, **Makefile targets**, **Tekton PipelineRuns**,
42+
or any other CI/build system file that builds Go binaries importing `pkg/scheme` or any
43+
`//go:build distro` gated code.
44+
45+
The core principle: `GOTAGS=distro` must flow unbroken from the build system into `go build`.
46+
Every layer in the chain is a potential break point.
47+
48+
7. **Dockerfile missing GOTAGS support** - Any Dockerfile that compiles a Go binary which imports
49+
`pkg/scheme` (or any package with `//go:build distro` companion files) must declare `ARG GOTAGS`
50+
in the builder stage and pass `-tags "${GOTAGS}"` to `go build`. Without this, `*_ocp.go` files
51+
are silently skipped - causing missing scheme registrations, CRD watch failures, or runtime
52+
crashes. Two valid patterns:
53+
- `ARG GOTAGS=""` when the caller always supplies the value (Makefile / generic CI).
54+
- `ARG GOTAGS="distro"` when the Dockerfile is exclusively used for distro builds (Konflux).
55+
Canonical references: `llmisvc-controller.Dockerfile` (Makefile pattern),
56+
`Dockerfiles/llmisvc-controller.Dockerfile.konflux` (Konflux pattern).
57+
58+
8. **Build system invocation not passing GOTAGS** - Every invocation of a Dockerfile covered by
59+
rule 7 must pass `GOTAGS=distro` through that build system's mechanism for Docker build
60+
arguments. The mechanism varies by system - check each caller type in the diff:
61+
- **Makefile**: `--build-arg GOTAGS=${GOTAGS}` on the `buildx build` call.
62+
Canonical references: `docker-build` and `docker-build-llmisvc` in `Makefile`.
63+
- **Tekton PipelineRun** (`.tekton/*.yaml`): `build-args: ["GOTAGS=distro"]` in `spec.params`.
64+
Canonical reference: `odh-kserve-llmisvc-controller-pull-request.yaml` in `.tekton/`.
65+
- **Konflux Dockerfiles** (`Dockerfiles/*.Dockerfile.konflux`): prefer `ARG GOTAGS="distro"`
66+
as the default so the pipeline does not need to pass it explicitly.
67+
Canonical reference: `Dockerfiles/llmisvc-controller.Dockerfile.konflux`.
68+
If a new build system is introduced, apply the same principle: locate the equivalent of
69+
`--build-arg` for that system and verify GOTAGS reaches the compiler.
70+
3971
## Exemptions - do not flag
4072

4173
- Files under a `distro/` sub-package (e.g. `pkg/controller/.../distro/controller_rbac_ocp.go`)

.tekton/kserve-controller-pull.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ spec:
3232
- name: additional-tags
3333
value:
3434
- 'master-pr-{{revision}}'
35+
- name: build-args
36+
value:
37+
- "GOTAGS=distro"
3538
- name: pipeline-type
3639
value: pull-request
3740
- name: enable-group-testing

Dockerfile

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,12 @@ RUN --mount=type=cache,target=/go/pkg/mod \
1313
FROM deps AS builder
1414

1515
ARG CMD=manager
16+
ARG GOTAGS=""
1617
COPY cmd/${CMD}/ cmd/${CMD}/
1718
COPY pkg/ pkg/
1819
RUN --mount=type=cache,target=/go/pkg/mod \
1920
--mount=type=cache,target=/root/.cache/go-build \
20-
CGO_ENABLED=0 GOOS=linux GOFLAGS=-mod=readonly go build -a -o manager ./cmd/${CMD}
21+
CGO_ENABLED=0 GOOS=linux GOFLAGS=-mod=readonly go build -a -tags "${GOTAGS}" -o manager ./cmd/${CMD}
2122

2223
# ---- License stage (parallel with build on BuildKit) ----
2324
FROM deps AS license

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ bump-version:
469469

470470
# Build the docker image
471471
docker-build:
472-
${ENGINE} buildx build ${ARCH} . -t ${KO_DOCKER_REPO}/${CONTROLLER_IMG}
472+
${ENGINE} buildx build ${ARCH} --build-arg GOTAGS=${GOTAGS} . -t ${KO_DOCKER_REPO}/${CONTROLLER_IMG}
473473
@echo "updating kustomize image patch file for manager resource"
474474

475475
# Use perl instead of sed to avoid OSX/Linux compatibility issue:

cmd/manager/main.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,6 @@ import (
3737
"sigs.k8s.io/controller-runtime/pkg/webhook"
3838
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
3939

40-
routev1 "github.com/openshift/api/route/v1"
41-
4240
"github.com/kserve/kserve/pkg/apis/serving/v1alpha1"
4341
"github.com/kserve/kserve/pkg/apis/serving/v1beta1"
4442
"github.com/kserve/kserve/pkg/constants"
@@ -149,8 +147,8 @@ func main() {
149147
setupLog.Error(err, "unable to register API schemes")
150148
os.Exit(1)
151149
}
152-
if err := routev1.AddToScheme(mgr.GetScheme()); err != nil {
153-
setupLog.Error(err, "unable to add routev1 APIs to scheme")
150+
if err := registerDistroSchemes(mgr.GetScheme()); err != nil {
151+
setupLog.Error(err, "unable to register distro-specific API schemes")
154152
os.Exit(1)
155153
}
156154

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
//go:build !distro
2+
3+
/*
4+
Copyright 2026 The KServe Authors.
5+
6+
Licensed under the Apache License, Version 2.0 (the "License");
7+
you may not use this file except in compliance with the License.
8+
You may obtain a copy of the License at
9+
10+
http://www.apache.org/licenses/LICENSE-2.0
11+
12+
Unless required by applicable law or agreed to in writing, software
13+
distributed under the License is distributed on an "AS IS" BASIS,
14+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
See the License for the specific language governing permissions and
16+
limitations under the License.
17+
*/
18+
19+
package main
20+
21+
import "k8s.io/apimachinery/pkg/runtime"
22+
23+
// registerDistroSchemes is a no-op in upstream builds.
24+
func registerDistroSchemes(_ *runtime.Scheme) error {
25+
return nil
26+
}

cmd/manager/main_schemes_ocp.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
//go:build distro
2+
3+
/*
4+
Copyright 2026 The KServe Authors.
5+
6+
Licensed under the Apache License, Version 2.0 (the "License");
7+
you may not use this file except in compliance with the License.
8+
You may obtain a copy of the License at
9+
10+
http://www.apache.org/licenses/LICENSE-2.0
11+
12+
Unless required by applicable law or agreed to in writing, software
13+
distributed under the License is distributed on an "AS IS" BASIS,
14+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
See the License for the specific language governing permissions and
16+
limitations under the License.
17+
*/
18+
19+
package main
20+
21+
import (
22+
routev1 "github.com/openshift/api/route/v1"
23+
"k8s.io/apimachinery/pkg/runtime"
24+
)
25+
26+
// registerDistroSchemes registers OCP-specific API schemes (e.g. OpenShift Routes).
27+
func registerDistroSchemes(s *runtime.Scheme) error {
28+
return routev1.AddToScheme(s)
29+
}

pkg/credentials/credentials_suite_test.go

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,9 @@ package credentials
1818

1919
import (
2020
"os"
21-
"path/filepath"
2221
"testing"
2322

2423
"k8s.io/client-go/kubernetes"
25-
"k8s.io/client-go/kubernetes/scheme"
2624
"k8s.io/client-go/rest"
2725
"sigs.k8s.io/controller-runtime/pkg/client"
2826

@@ -36,16 +34,13 @@ var (
3634
)
3735

3836
func TestMain(m *testing.M) {
39-
crdDirectoryPaths := []string{
40-
filepath.Join(pkgtest.ProjectRoot(), "test", "crds"),
41-
}
42-
t := pkgtest.SetupEnvTest(crdDirectoryPaths)
37+
t := pkgtest.NewEnvTest().BuildEnvironment()
4338
var err error
4439
if cfg, err = t.Start(); err != nil {
4540
log.Error(err, "Failed to start testing panel")
4641
}
4742

48-
if c, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}); err != nil {
43+
if c, err = client.New(cfg, client.Options{Scheme: t.Scheme}); err != nil {
4944
log.Error(err, "Failed to start client")
5045
}
5146
if clientset, err = kubernetes.NewForConfig(cfg); err != nil {

pkg/scheme/register.go

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
wvav1alpha1 "github.com/llm-d/llm-d-workload-variant-autoscaler/api/v1alpha1"
2222
otelv1beta1 "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1"
2323
"github.com/pkg/errors"
24-
monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
2524
istioclientv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1"
2625
appsv1 "k8s.io/api/apps/v1"
2726
autoscalingv2 "k8s.io/api/autoscaling/v2"
@@ -45,6 +44,10 @@ import (
4544

4645
type addToSchemeFunc func(scheme *runtime.Scheme) error
4746

47+
// distroSchemes collects distribution-specific scheme registration functions.
48+
// Populated via init() in build-tagged companion files (register_ocp.go).
49+
var distroSchemes []addToSchemeFunc
50+
4851
// AddKServeAPIs registers all KServe APIs.
4952
func AddKServeAPIs(s *runtime.Scheme) error {
5053
return addAll(s,
@@ -106,12 +109,6 @@ func AddOpenTelemetryAPIs(s *runtime.Scheme) error {
106109
return addAll(s, otelv1beta1.AddToScheme)
107110
}
108111

109-
// AddMonitoringAPIs registers Prometheus Operator monitoring APIs (PodMonitor, ServiceMonitor).
110-
// The scheme registration is unconditional; actual CRD availability is checked at watch setup time.
111-
func AddMonitoringAPIs(s *runtime.Scheme) error {
112-
return addAll(s, monitoringv1.AddToScheme)
113-
}
114-
115112
// AddControllerAPIs registers the baseline controller APIs used by production and tests.
116113
func AddControllerAPIs(s *runtime.Scheme) error {
117114
return addAll(s,
@@ -122,19 +119,18 @@ func AddControllerAPIs(s *runtime.Scheme) error {
122119

123120
// AddLLMISVCAPIs registers API groups required by the llmisvc manager.
124121
func AddLLMISVCAPIs(s *runtime.Scheme) error {
125-
return addAll(s,
122+
return addAll(s, append([]addToSchemeFunc{
126123
AddControllerAPIs,
127124
AddGatewayAPIs,
128125
AddLeaderWorkerSetAPIs,
129-
AddMonitoringAPIs,
130126
AddKedaAPIs,
131127
AddWVAAPIs,
132-
)
128+
}, distroSchemes...)...)
133129
}
134130

135131
// AddAll registers all API groups supported by KServe managers and envtest suites.
136132
func AddAll(s *runtime.Scheme) error {
137-
return addAll(s,
133+
return addAll(s, append([]addToSchemeFunc{
138134
AddControllerAPIs,
139135
AddGatewayAPIs,
140136
AddLeaderWorkerSetAPIs,
@@ -143,8 +139,7 @@ func AddAll(s *runtime.Scheme) error {
143139
AddKedaAPIs,
144140
AddWVAAPIs,
145141
AddOpenTelemetryAPIs,
146-
AddMonitoringAPIs,
147-
)
142+
}, distroSchemes...)...)
148143
}
149144

150145
func addAll(s *runtime.Scheme, fns ...addToSchemeFunc) error {

pkg/scheme/register_ocp.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
//go:build distro
2+
3+
/*
4+
Copyright 2026 The KServe Authors.
5+
6+
Licensed under the Apache License, Version 2.0 (the "License");
7+
you may not use this file except in compliance with the License.
8+
You may obtain a copy of the License at
9+
10+
http://www.apache.org/licenses/LICENSE-2.0
11+
12+
Unless required by applicable law or agreed to in writing, software
13+
distributed under the License is distributed on an "AS IS" BASIS,
14+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
See the License for the specific language governing permissions and
16+
limitations under the License.
17+
*/
18+
19+
package scheme
20+
21+
import (
22+
monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
23+
"k8s.io/apimachinery/pkg/runtime"
24+
)
25+
26+
func init() {
27+
distroSchemes = append(distroSchemes, AddMonitoringAPIs)
28+
}
29+
30+
// AddMonitoringAPIs registers Prometheus Operator monitoring APIs (PodMonitor, ServiceMonitor).
31+
func AddMonitoringAPIs(s *runtime.Scheme) error {
32+
return addAll(s, monitoringv1.AddToScheme)
33+
}

0 commit comments

Comments
 (0)