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

ability to specify init container on per deployment basis #2611

Closed
RafalSkolasinski opened this issue Nov 4, 2020 — with Board Genius Sync · 7 comments
Closed
Assignees
Projects
Milestone

Comments

Copy link
Contributor

Currently init containers can only be specified globally. It would be good if one can specify init container on per deployment basis.

@RafalSkolasinski RafalSkolasinski added the triage Needs to be triaged and prioritised accordingly label Nov 4, 2020
@ukclivecox
Copy link
Contributor

You can add your own initContainer in the connected PodSpec. But if you are using a prepackaged server you will still get its initContainer added by default which may conflict.

@ukclivecox ukclivecox removed the triage Needs to be triaged and prioritised accordingly label Nov 5, 2020
@RafalSkolasinski RafalSkolasinski self-assigned this Dec 14, 2020
@RafalSkolasinski RafalSkolasinski added this to To do in 1.6.0 via automation Dec 14, 2020
@RafalSkolasinski RafalSkolasinski added this to the 1.6 milestone Dec 14, 2020
@RafalSkolasinski
Copy link
Contributor Author

Actually, tested it just now and I only got one init container with prepackaged server. This is most probably because name matched the pattern. The example I came up with is

apiVersion: machinelearning.seldon.io/v1
kind: SeldonDeployment
metadata:
  name: example-test
spec:
  name: iris
  predictors:
  - componentSpecs:
    - spec:
        containers:
        - name: classifier
          volumeMounts:
          - mountPath: /mnt/models
            name: classifier-provision-location
            readOnly: true
    
        initContainers:
        - name: classifier-model-initializer
          image: rafalskolasinski/storage-initializer:v0.4.0
          imagePullPolicy: IfNotPresent
          args:
          - s3://sklearn/iris
          - /mnt/models
    
          envFrom:
          - secretRef:
              name: seldon-init-container-secret
    
          terminationMessagePath: /dev/termination-log
          terminationMessagePolicy: File
    
          volumeMounts:
          - mountPath: /mnt/models
            name: classifier-provision-location
    
        volumes:
        - emptyDir: {}
          name: classifier-provision-location
    
    graph:
      children: []
      implementation: SKLEARN_SERVER
      modelUri: s3://sklearn/iris
      name: classifier
    name: default
    replicas: 1

(which only customises the secret and image)

@RafalSkolasinski
Copy link
Contributor Author

Adding above example as part of #2055 should solve this issue.

@RafalSkolasinski
Copy link
Contributor Author

Thinking about it more, we should consider this issue close only when it will be possible to specify image used for initContainer per deployment in a more elegant fashion than above.

@RafalSkolasinski
Copy link
Contributor Author

RafalSkolasinski commented Jan 7, 2021

To be precise, I would like to allow users to be able to specify a following

apiVersion: machinelearning.seldon.io/v1
kind: SeldonDeployment
metadata:
  name: custom-init-image
spec:
  name: iris
  predictors:
  - componentSpecs:
    - spec:
        initContainers:
        - name: classifier-model-initializer
          image: seldonio/storage-initializer:v1.6.0
    graph:
      implementation: SKLEARN_SERVER
      modelUri: gs://seldon-models/sklearn/iris
      name: classifier
    name: default
    replicas: 1

With the assumption that all valid init container images provided these way would take two arguments:

  • source
  • destination

@RafalSkolasinski
Copy link
Contributor Author

This should in principle work like this but currently operator fails to create a proper deployments: #2821

@axsaucedo axsaucedo changed the title ability to specify init container on per deployment basis OSS-145: ability to specify init container on per deployment basis Apr 26, 2021
@axsaucedo axsaucedo changed the title OSS-145: ability to specify init container on per deployment basis ability to specify init container on per deployment basis Apr 28, 2021
@RafalSkolasinski
Copy link
Contributor Author

closed by #2937

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
1.6.0
To do
Development

No branches or pull requests

2 participants