Add kubernetes.YAML support to ramalama serve #327
Conversation
rhatdan
changed the title
|
@ygalblum @umohnani8 @mrunalp @haircommander PTAL to make sure my kube YAML looks ok |
|
LGTM |
docs/ramalama-serve.1.md
Outdated
| kind: Pod | ||
| metadata: | ||
| labels: | ||
| app: tini-pod |
There was a problem hiding this comment.
Don't I have to name these different then the tini container?
docs/ramalama-serve.1.md
Outdated
| args: ['--port', '8080', '-m', '/run/model'] | ||
| ports: | ||
| - containerPort: 8080 | ||
| hostPort: 8080 |
There was a problem hiding this comment.
I think it's best not to the hostPort
There was a problem hiding this comment.
So only do hostPort if different?
There was a problem hiding this comment.
In most cases hostPort is not really used in K8S. Endpoints are exposed via Services. Maybe consider setting the port's name
docs/ramalama-serve.1.md
Outdated
| - containerPort: 8080 | ||
| hostPort: 8080 | ||
| volumeMounts: | ||
| - mountPath: /run/model |
There was a problem hiding this comment.
This indentation works, but we should keep the way arrays are indented consistent
| name: dri | ||
| volumes: | ||
| - name model | ||
| hostPath: |
There was a problem hiding this comment.
I don't think hostPath will be used for these type of data. It will probably use a persistent volume
There was a problem hiding this comment.
Yes i am hoping to change this to use an image at some point. But if you could give me an example of what it should look like?
|
|
||
| return mounts + volumes | ||
|
|
||
| def kube(self, model, args, exec_args): |
There was a problem hiding this comment.
My experience with generating YAML files with templates has always been a poor one. There was always some indentation that got botched. I think a better solution is to create a dict and dump it using the pyyaml library by calling print(yaml.dump(d))
There was a problem hiding this comment.
Cool, do you have an example of a template for a kube.yaml?
docs/ramalama-serve.1.md
Outdated
| # | ||
| # Created with ramalama-0.0.17 | ||
| apiVersion: v1 | ||
| kind: Pod |
There was a problem hiding this comment.
Come to think about it I think a Deployment will be more useful than a Pod
Fixes: containers#183 Signed-off-by: Daniel J Walsh <[email protected]>
Fixes: #183