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

allow use of multiple image pull secrets in KubernetesAgent, DaskKubernetesEnvironment #3596

Merged
merged 4 commits into from
Oct 30, 2020
Merged

allow use of multiple image pull secrets in KubernetesAgent, DaskKubernetesEnvironment #3596

merged 4 commits into from
Oct 30, 2020

Conversation

jameslamb
Copy link

@jameslamb jameslamb commented Oct 29, 2020

Summary

Today, you can customize the value of imagePullSecrets (the secrets used to pull docker images) used by KubernetesAgent and DaskKubernetesAgent. However, prefect only allows you to specify a single secret.

This PR proposes expanding that, to allow multiple image pull secrets to be provided.

Changes

Changes KubernetesAgent and DaskKubernetesAgent to accept multiple imagPullSecrets values. What I'm proposing here is backwards compatible with how prefect works today

  • one secret: "my-secret"
  • 2 secrets: "my-secret,other-secret"

Importance

I started looking into this because of this message that Docker sent out last night:

You are receiving this email because of a policy change to Docker products and services you use. On Monday, November 2, 2020 at 9am Pacific Standard Time, Docker will begin enforcing rate limits on container pulls for Anonymous and Free users. Anonymous (unauthenticated) users will be limited to 100 container image pulls every six hours, and Free (authenticated) users will be limited to 200 container image pulls every six hours, when enforcement is fully implemented...

More details are available in this blog post.

This change means that people using prefect kubernetes components that pull public images from Docker Hub might want to now make authenticated (instead of anonymous) calls to Docker Hub, to reduce the risk of hitting that rate limit.

This PR makes it possible to run these components with multiple secrets so that, for example, an agent could pull images from either Docker Hub or a private registry like AWS ECR.

Checklist

This PR:

  • adds new tests (if appropriate)
  • adds a change file in the changes/ directory (if appropriate)
  • updates docstrings for any new functions or function arguments, including docs/outline.toml for API reference docs (if appropriate)

Notes for reviewers

  • I searched for places to update like this:
    git grep -i imagePullSec src/
    git grep -i image_pull_sec src/
  • I know that environments might go away in the future (Remove the need for Environment classes entirely #2928) but thought they should continue to get patches like this until they do. Let me know if you disagree and I can remove the DaskKubernetesEnvironment code from this

Questions for reviewers

  • is it intentional that there is no "customize imagePullSecrets" functionality for KubernetesJobEnvironment? Is the thinking the idea there that the customization should just go into job_spec_file instead of adding new environment variables / kwargs to that class?

Thanks for your time and consideration.

@codecov
Copy link

codecov bot commented Oct 29, 2020

Codecov Report

Merging #3596 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

@jameslamb jameslamb mentioned this pull request Oct 29, 2020
3 tasks
changes/pr3596.yaml Outdated Show resolved Hide resolved
Copy link

@jcrist jcrist left a comment

Choose a reason for hiding this comment

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

Thanks @jameslamb! This looks good to me.

Note that with the new KubernetesRun pattern (we should be rolling out docs/release in the next couple weeks) this is already possible - the full k8s job spec is exposed and configurable on either agent or as part of the KubernetesRun object itself.

@jameslamb
Copy link
Author

Thanks @jameslamb! This looks good to me.

Note that with the new KubernetesRun pattern (we should be rolling out docs/release in the next couple weeks) this is already possible - the full k8s job spec is exposed and configurable on either agent or as part of the KubernetesRun object itself.

oooo cool! First I've heard of that, I'm looking forward to trying it out.

@joshmeek joshmeek merged commit e5553ee into PrefectHQ:master Oct 30, 2020
@jameslamb jameslamb deleted the feat/multiple-image-pull-secrets branch October 30, 2020 17:04
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.

None yet

4 participants