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

Add support for default image pull secret name #921

Merged
merged 7 commits into from Dec 15, 2021

Conversation

fgalind1
Copy link
Contributor

@fgalind1 fgalind1 commented Oct 28, 2021

Currently it's supported to define ImagePullSecrets in the runner spec e.g as a RunnerDeployment. However the controller can specify a default runner image.

If this image is on a private registry it requires the image pull secret. This PR adds this option also to the controller to be consistent with supporting a default runner image. This makes it much
easier when using runner images from private registries

Fixes #896

Currently it's supported to define imagePullSecrets in the runner spec
e.g as a RunnerDeployment. However the controller can specify a default
runner image.

If this image is on a private registry it requires the
image pull secret. This PR adds this option also to the controller to be
consistent with supporting a default runner image. This makes it much
easier when using runner images from private registries
@stale
Copy link

stale bot commented Nov 28, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 28, 2021
@fgalind1
Copy link
Contributor Author

still needed

@stale stale bot removed the stale label Nov 29, 2021
@mumoshu mumoshu added this to the v0.21.0 milestone Nov 29, 2021
@mumoshu
Copy link
Collaborator

mumoshu commented Nov 29, 2021

@fgalind1 Sorry for the delay and thanks for your contribution! I'm going to review/merge this towards our next feature release(0.21.0)

@fgalind1 fgalind1 force-pushed the support-default-image-pull-secrets branch from 944c8aa to 7590eed Compare December 13, 2021 18:42
Comment on lines 879 to 881
pod.Spec.ImagePullSecrets = []corev1.LocalObjectReference{
{Name: imagePullSecret},
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
pod.Spec.ImagePullSecrets = []corev1.LocalObjectReference{
{Name: imagePullSecret},
}
pod.Spec.ImagePullSecrets = append(pod.Spec.ImagePullSecrets, []corev1.LocalObjectReference{
{Name: imagePullSecret},
})

otherwise pod.spec.ImagePullSecrets always contain one element which correlate to the last element in defualtRUnnerImagePullSecrets, which won't be what you want :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks good catch - fixed in ffa0d58

Copy link
Collaborator

@mumoshu mumoshu left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks a lot for your efforts and contribution @fgalind1! ☺️

@mumoshu mumoshu merged commit 9bb21ae into actions:master Dec 15, 2021
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.

Support default imagePullSecrets in runner pod for private docker registries
2 participants