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

setting imagePullSecrets for workers #652

Merged
merged 5 commits into from
Jul 1, 2024

Conversation

chokosabe
Copy link
Contributor

Setting the imagepullsecrets. Not sure this is the best location to do this in.

Copy link
Member

@mwylde mwylde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great start but need a little more work

k8s/arroyo/templates/configmap.yaml Outdated Show resolved Hide resolved
k8s/arroyo/templates/configmap.yaml Outdated Show resolved Hide resolved
@chokosabe
Copy link
Contributor Author

Updates pushed up. Resolved all the changes.

Copy link
Member

@mwylde mwylde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still isn't quite working. It might be useful to try testing it locally with docker desktop. With a local k8s environment, you can test it by creating a values.yaml file like this:

artifactUrl: "/tmp/arroyo-test"
checkpointUrl: "/tmp/arroyo-test"

volumes:
  - name: checkpoints
    hostPath:
      path: /tmp/arroyo-test
      type: DirectoryOrCreate

volumeMounts:
  - name: checkpoints
    mountPath: /tmp/arroyo-test

Then building an image from your branch and deploying it locally:

$ docker build . -f docker/Dockerfile -t ghcr.io/arroyosystems/arroyo:tip --target arroyo
$ helm install arroyo  k8s/arroyo -f values.yaml 

command: ["/app/arroyo-bin --config-dir /config worker"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still isn't quite right. With this, trying to run this in helm gives the error

Configuration is invalid!
  • invalid type: found sequence, expected a string for key "default.kubernetes-scheduler.worker.command" in /root/.config/arroyo/config.yaml YAML file

It should be restored to what it currently is in master.

@@ -59,6 +59,10 @@ data:
{{- end }}
image: "{{ .Values.worker.image.repository }}:{{ .Values.worker.image.tag }}"
image-pull-policy: "{{ .Values.worker.image.pullPolicy }}"
image-pull-secrets:
{{- with .Values.imagePullSecrets }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image_pull_secrets is defined as a string, but here it's passed in an object which fails:

Configuration is invalid!
  • invalid type: found unit, expected a string for key "default.kubernetes-scheduler.worker.image-pull-secrets" in /root/.config/arroyo/config.yaml YAML file

@@ -64,6 +64,7 @@ resource-mode = "per-slot"
name-prefix = "arroyo"
image = "ghcr.io/arroyosystems/arroyo:latest"
image-pull-policy = "IfNotPresent"
image-pull-secrets = "IfNotPresent"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a sensible default for this?

@mwylde mwylde force-pushed the set-imagepullsecrets-worker branch from 6f33f34 to e3c8c38 Compare July 1, 2024 22:13
Copy link
Member

@mwylde mwylde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! I've gone ahead and rebased on master and fixed a couple of issues so that we can get this into the next release.

@mwylde mwylde enabled auto-merge (squash) July 1, 2024 22:14
@mwylde mwylde merged commit 46dde68 into ArroyoSystems:master Jul 1, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants