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

Use GHCR docker image by default #39

Merged
merged 1 commit into from
Jun 12, 2024

Conversation

ianb-mp
Copy link
Contributor

@ianb-mp ianb-mp commented Jun 10, 2024

Make it easer for users (rather than developers) to get started by making GHCR the default image source.

  • update deployment/daemonset.yaml to reference GHCR rather than localhost
  • update README to make building the Docker image an optional step. Also use LF instead of CR/LF line endings
  • use fully qualified image path in Dockerfile (helpful for Podman)
  • fix missing configmap (caused by Remove driver version check  #37)

Fix daemonset, missing configmap
@@ -1,12 +1,12 @@
FROM golang:alpine as builder
FROM docker.io/golang:alpine as builder
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's the reason behind this? I know we can do short hand and configure local container runtime.

I don't mind being more specific, just curious,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the general case, I think it's always better to be explicit about the container image source to ensure you get exactly the image you intend. In the case of Podman, by default it will throw an error about ambiguous image reference (it can be configured to do autocompletion).

Copy link
Collaborator

@Eoghan1232 Eoghan1232 left a comment

Choose a reason for hiding this comment

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

1 comment, otherwise lgtm

@Eoghan1232
Copy link
Collaborator

@zeeke ptal

Copy link
Member

@zeeke zeeke left a comment

Choose a reason for hiding this comment

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

LGTM

@Eoghan1232 Eoghan1232 merged commit d4be6f7 into k8snetworkplumbingwg:master Jun 12, 2024
9 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.

3 participants