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

upgrading from 1.1 to 1.2 with existing sdep leads to volume mount error #2017

Closed
ryandawsonuk opened this issue Jun 26, 2020 · 1 comment · Fixed by #2019
Closed

upgrading from 1.1 to 1.2 with existing sdep leads to volume mount error #2017

ryandawsonuk opened this issue Jun 26, 2020 · 1 comment · Fixed by #2019
Labels
bug triage Needs to be triaged and prioritised accordingly
Milestone

Comments

@ryandawsonuk
Copy link
Contributor

ryandawsonuk commented Jun 26, 2020

Seems to stem from #1863

Problem appears to be that mutating webhook applies the volume mount to the sdep. So my sdep (iris example) contains:

          volumeMounts:
          - mountPath: /etc/podinfo
            name: podinfo

The mutating webhook doesn't modify this on the upgrade to 1.2, if it runs at all (searching logs it seems it has not). The result is an error in the operator when trying to apply both mounts:

2020-06-26T15:03:48.105Z        ERROR   controller-runtime.controller   Reconciler error        {"controller": "seldon-controller-manager", "request": "seldon/iris", "error": "Deployment.apps \"iris-default-0-iris-container\" is invalid: [spec.template.spec.containers[0].volumeMounts[0].name: Not found: \"podinfo\", spec.template.spec.containers[0].volumeMounts[1].mountPath: Invalid value: \"/etc/podinfo\": must be unique]"}
github.com/go-logr/zapr.(*zapLogger).Error
        /go/pkg/mod/github.com/go-logr/zapr@v0.1.0/zapr.go:128
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler
        /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.5.0/pkg/internal/controller/controller.go:258
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem
        /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.5.0/pkg/internal/controller/controller.go:232
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).worker
        /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.5.0/pkg/internal/controller/controller.go:211
k8s.io/apimachinery/pkg/util/wait.JitterUntil.func1
        /go/pkg/mod/k8s.io/apimachinery@v0.17.2/pkg/util/wait/wait.go:152
k8s.io/apimachinery/pkg/util/wait.JitterUntil
        /go/pkg/mod/k8s.io/apimachinery@v0.17.2/pkg/util/wait/wait.go:153
k8s.io/apimachinery/pkg/util/wait.Until
        /go/pkg/mod/k8s.io/apimachinery@v0.17.2/pkg/util/wait/wait.go:88

We could fix by adjusting this check (and any other volFound checks) -

to check for both the old and new volume mount names.

We should not change the mount path as that is used in the wrapper and then models would have to be rewrapped.

@ryandawsonuk ryandawsonuk added bug triage Needs to be triaged and prioritised accordingly labels Jun 26, 2020
@ryandawsonuk ryandawsonuk added this to the 1.2 milestone Jun 26, 2020
@ryandawsonuk
Copy link
Contributor Author

Upgrade tests don't pick up on this because it repeatedly checks http requests. But the existing pods keep running.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug triage Needs to be triaged and prioritised accordingly
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant