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 ability to pass docker config and included registry credentials to build pods' docker clients, test builds and pushes, and add dockerApi.[extraArgs|extraFiles] to help testing #36

Merged
merged 7 commits into from
Jul 10, 2023

Conversation

consideRatio
Copy link
Contributor

@consideRatio consideRatio commented Jun 5, 2023

With binderhub-service being a Helm chart deploying to k8s, we absolutely require the ability to push the built artifact to a container registry somewhere. This enables the chart to create a k8s Secret with such credentials in a docker config format that can be mounted to the build pods via c.KubernetesBuildExecutor.push_secret. Users of the binderhub-service chart doesn't have to configure that manually, its instead set automatically.

buildPodsRegistryCredentials

buildPodsRegistryCredentials should be provided with server, username, and password for the container registry. For a GCP based artifact-registry, this can be for example...

config:
  BinderHub:
    use_registry: true
    image_prefix: europe-west1-docker.pkg.dev/binderhub-service-development/binderhub-service/
buildPodsRegistryCredentials:
  server: https://europe-west1-docker.pkg.dev
  # This GCP ServiceAccount is configured with:
  # - roles/artifactregistry.createOnPushWriter
  #
  username: _json_key
  password: |
    {
      "type": "service_account",
      "project_id": "binderhub-service-development",
      "private_key_id": "3ca6cb965bf9d8a23ee07220c6bfcdf70a2b4234",
      "private_key": "CENSORED",
      "client_email": "artifact-registry-push@binderhub-service-development.iam.gserviceaccount.com",
      "client_id": "101939388990083667377",
      "auth_uri": "https://accounts.google.com/o/oauth2/auth",
      "token_uri": "https://oauth2.googleapis.com/token",
      "auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs",
      "client_x509_cert_url": "https://www.googleapis.com/robot/v1/metadata/x509/artifact-registry-push%40binderhub-service-development.iam.gserviceaccount.com",
      "universe_domain": "googleapis.com"
    }

buildPodsDockerConfig

This may not be relevant for us to use, but was added for parity with binderhub chart to some degree.

buildPodsDockerConfig:
  someDockerConfigRecognizableKey: and-value

dockerApi.[extraArgs|extraFiles]

In order to setup tests with build/push, I needed to configure the docker daemon runnin on the host node to tolerate interacting with HTTP based docker registries like the one we have. Instead of doing like in jupyterhub/binderhub where a configmap is created and mounted etc in a way that is very hard to follow, I do something that is just quite hard to follow by introducing extraFiles taken from z2jh.

Tests build/push

@consideRatio consideRatio marked this pull request as draft June 5, 2023 09:19
@consideRatio consideRatio force-pushed the pr/mash-backup branch 4 times, most recently from 1776567 to c6f542a Compare June 5, 2023 11:51
@GeorgianaElena GeorgianaElena reopened this Jul 7, 2023
dev-config.yaml Outdated Show resolved Hide resolved
@consideRatio consideRatio changed the title Add ability to pass registry credentials to build pods' docker clients Add ability to pass docker config and included registry credentials to build pods' docker clients Jul 8, 2023
@consideRatio consideRatio force-pushed the pr/mash-backup branch 3 times, most recently from 621c919 to ce1f141 Compare July 8, 2023 17:54
@consideRatio consideRatio force-pushed the pr/mash-backup branch 3 times, most recently from c9fb963 to 00a549e Compare July 8, 2023 19:45
@consideRatio consideRatio changed the title Add ability to pass docker config and included registry credentials to build pods' docker clients Add ability to pass docker config and included registry credentials to build pods' docker clients, test builds and pushes, and add dockerApi.[extraArgs|extraFiles] to help testing Jul 8, 2023
@consideRatio consideRatio marked this pull request as ready for review July 8, 2023 19:48
@consideRatio
Copy link
Contributor Author

consideRatio commented Jul 8, 2023

@GeorgianaElena I think this is good to go finally, its quite a messy PR because it introduced dockerApi.extraFiles for example, but that is copy-pasted from z2jh where it has been very successfully used reliably to inject files into pods.

I suggest we go for a merge and iterating from there with fixes if needed.

@GeorgianaElena
Copy link
Member

Thanks @consideRatio!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Archived in project
Development

Successfully merging this pull request may close these issues.

ci: improve our tests to also trigger a build and push to a registry
2 participants