configurable prepack images

[ryandawsonuk]

for #959

[ryandawsonuk]

I think we need access to the configmap entries in the webhook, since there's a prepackaged model server scenario where some parts of the container spec might be set (e.g. resources) without an image. Then we have to fill in the image. But we don't currently have access to the configmap in the webhook

[ryandawsonuk]

I tried going the environment variable route by setting this in the manager.yaml:

        - name: PREDICTORS_CONFIG
          valueFrom:
            configMapKeyRef:
              name: config
              key: predictor_servers

And then loading the json from that env var string. But then the docker-build blows up, presumably with kubebuilder:

        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x985d4f]

goroutine 1 [running]:
go/types.(*Checker).handleBailout(0xc00078c000, 0xc003a0f748)
        /usr/local/go/src/go/types/check.go:240 +0x98
panic(0xae8e20, 0x111a5c0)
        /usr/local/go/src/runtime/panic.go:679 +0x1b2
sync.(*Mutex).Lock(...)
        /usr/local/go/src/sync/mutex.go:74
sigs.k8s.io/controller-tools/pkg/loader.(*loader).typeCheck.func1(0xc0038a8001, 0x35, 0x0, 0x0, 0x0)
        /home/ryan/go/pkg/mod/sigs.k8s.io/controller-tools@v0.2.0/pkg/loader/loader.go:255 +0xaf
sigs.k8s.io/controller-tools/pkg/loader.importerFunc.Import(0xc0016ff970, 0xc0038a8001, 0x35, 0x0, 0x0, 0xbb2700)
        /home/ryan/go/pkg/mod/sigs.k8s.io/controller-tools@v0.2.0/pkg/loader/loader.go:361 +0x3a
go/types.(*Checker).importPackage(0xc00078c000, 0x34478d, 0xc0038a8001, 0x35, 0xc00032b9a0, 0x39, 0x1)
        /usr/local/go/src/go/types/resolver.go:161 +0x612
go/types.(*Checker).collectObjects(0xc00078c000)
        /usr/local/go/src/go/types/resolver.go:256 +0x15d9
go/types.(*Checker).checkFiles(0xc00078c000, 0xc0038150c0, 0x8, 0x8, 0x0, 0x0)
        /usr/local/go/src/go/types/check.go:252 +0xad
go/types.(*Checker).Files(...)
        /usr/local/go/src/go/types/check.go:245
sigs.k8s.io/controller-tools/pkg/loader.(*loader).typeCheck(0xc000187ef0, 0xc0002fcfa0)
        /home/ryan/go/pkg/mod/sigs.k8s.io/controller-tools@v0.2.0/pkg/loader/loader.go:284 +0x3b5
sigs.k8s.io/controller-tools/pkg/loader.(*Package).NeedTypesInfo(0xc0002fcfa0)
        /home/ryan/go/pkg/mod/sigs.k8s.io/controller-tools@v0.2.0/pkg/loader/loader.go:96 +0x5b
sigs.k8s.io/controller-tools/pkg/loader.(*TypeChecker).check(0xc003a2cfc0, 0xc0002fcfa0)
        /home/ryan/go/pkg/mod/sigs.k8s.io/controller-tools@v0.2.0/pkg/loader/refs.go:249 +0x1f8
sigs.k8s.io/controller-tools/pkg/loader.(*TypeChecker).Check(0xc0002fd0c0, 0xc0002fcfa0, 0xbe1270)
        /home/ryan/go/pkg/mod/sigs.k8s.io/controller-tools@v0.2.0/pkg/loader/refs.go:206 +0x90
sigs.k8s.io/controller-tools/pkg/deepcopy.(*ObjectGenCtx).GenerateForPackage(0xc003a0fb10, 0xc0002fcfa0, 0xc00382c000, 0x3147, 0x386e)
        /home/ryan/go/pkg/mod/sigs.k8s.io/controller-tools@v0.2.0/pkg/deepcopy/gen.go:199 +0x9a
sigs.k8s.io/controller-tools/pkg/deepcopy.Generator.Generate(0xc0000cadbb, 0x19, 0x0, 0x0, 0xc0000ee0f0, 0x1121cc0, 0xc0008de120)
        /home/ryan/go/pkg/mod/sigs.k8s.io/controller-tools@v0.2.0/pkg/deepcopy/gen.go:147 +0x1a5
sigs.k8s.io/controller-tools/pkg/genall.(*Runtime).Run(0xc00073f480, 0xc0000c39a0)
        /home/ryan/go/pkg/mod/sigs.k8s.io/controller-tools@v0.2.0/pkg/genall/genall.go:167 +0x16f
main.main.func1(0xc000141180, 0xc0000c39a0, 0x2, 0x2, 0x0, 0x0)
        /home/ryan/go/pkg/mod/sigs.k8s.io/controller-tools@v0.2.0/cmd/controller-gen/main.go:163 +0x99
github.com/spf13/cobra.(*Command).execute(0xc000141180, 0xc0000b6160, 0x2, 0x2, 0xc000141180, 0xc0000b6160)
        /home/ryan/go/pkg/mod/github.com/spf13/cobra@v0.0.3/command.go:762 +0x460
github.com/spf13/cobra.(*Command).ExecuteC(0xc000141180, 0xc0000c3940, 0x4, 0x0)
        /home/ryan/go/pkg/mod/github.com/spf13/cobra@v0.0.3/command.go:852 +0x2ea
github.com/spf13/cobra.(*Command).Execute(...)
        /home/ryan/go/pkg/mod/github.com/spf13/cobra@v0.0.3/command.go:800
main.main()
        /home/ryan/go/pkg/mod/sigs.k8s.io/controller-tools@v0.2.0/cmd/controller-gen/main.go:186 +0x2f5
make: *** [Makefile:81: generate] Error 2

Might be best to try to get access to the client.

[ryandawsonuk]

Actually that error seems to have been caused by commenting out a test. If I delete the test then I don't get that. Would be better to make the test actually work but I guess for that it will be necessary to mock the configmap lookup (whether the env var or client way).

[ryandawsonuk]

Turns out I don't need to pass through the client. So what I do need to do is work through the other uses for the prepack image code and put the test back in.

[ryandawsonuk]

Seems I didn't need to mock the client call either. It just requires that the configmap be installed in kind before running the tests.

/ok-to-test

[ryandawsonuk]

Tensorflow now seems to work but for SKLEARN the initContainer seems to have gone missing.

[axsaucedo]

/test integration

[ryandawsonuk]

Hmm, I managed to run tests locally by installing to the cluster before running the tests. In the cluster it's not doing it that way though.

[ryandawsonuk]

An example of using a configmap in a go test - https://github.com/kubeflow/kfserving/blob/master/pkg/controller/inferenceservice/controller_test.go

[ryandawsonuk]

Got the unit tests passing again, now just need to remove the unnecessary logging and double-check the e2e tests.

[ryandawsonuk]

/test integration
/approve
/lgtm

[seldondev]

@ryandawsonuk: you cannot LGTM your own PR.

In response to this:

/test integration
/approve
/lgtm

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.

[seldondev]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ryandawsonuk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [ryandawsonuk]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ryandawsonuk]

/test integration

[seldondev]

@ryandawsonuk: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
integration	b0f400c	link	/test integration

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. I understand the commands that are listed here.

[ryandawsonuk]

Not sure why the integration tests are failing here as they pass locally.

[ryandawsonuk]

So within a local kind cluster everything works. I've run e2e and tested manually. I've also built an image from the branch and run it in a GKE cluster and tested that manually and all works.

I'm stumped as to why the e2e tests in the jx cluster are failing. They're actually all failing, even the ones not related to prepackaged model servers. (The only ones passing are the ones where resource creation is meant to fail.)

[ryandawsonuk]

Hmm, now it seems to have been automatically merged without the e2e tests passing.

I have a theory now about what the problem was though. I think it's because we were on 0.5.1-SNAPSHOT when I opened the PR and we moved to 0.5.2-SNAPSHOT without me rebasing. The test output from in-cluster suggests the operator image getting built was using the newer version. Presumably the helm install was therefore installing a different image and perhaps running the old operator against the new configmap doesn't work.