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

feat: containerd configuration for mirror registry #292

Merged
merged 12 commits into from
Feb 6, 2024

Conversation

supershal
Copy link
Contributor

@supershal supershal commented Jan 17, 2024

Closes: #203

✔️ Create containerd mirror configuration for a registry
❌ Create Dynamic Credentials Provider configuration to use static mirror credentials
❌ Update Dynamic Credentials Provider configuration to use mirror credentials only with MirrorCredentialOnly strategy
✔️ Unit tests to check the generated kubeadm files
✔️ Updated documentation for global mirror

    variables:
      - name: clusterConfig
        value:
          imageRegistries:
            - url: https://my-registry.io
              credentials:
                secretRef:
                  name: my-registry-credentials
          globalImageRegistryMirror:
            - url: https://my-mirror.io
              credentials:
                secretRef:
                  name: my-mirror-CA-credentials

Future PRs:
Following Changes were making the current PR long and complicate to follow. I think we can integrate with konvoy using current changes in the PR.

  • Create Dynamic Credentials Provider configuration to use static mirror credentials
  • Update Dynamic Credentials Provider configuration to use mirror credentials only with MirrorCredentialOnly strategy

@supershal
Copy link
Contributor Author

Still running manual test with ECR registry mirror. Once the test completed, I will turn the draft PR to Ready.

dkoshkin
dkoshkin previously approved these changes Jan 17, 2024
Copy link
Contributor

@dkoshkin dkoshkin left a comment

Choose a reason for hiding this comment

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

Thank you for all the tests!

docs/content/customization/generic/image-registries.md Outdated Show resolved Hide resolved
docs/content/customization/generic/image-registries.md Outdated Show resolved Hide resolved
@supershal
Copy link
Contributor Author

supershal commented Jan 18, 2024

Tested that mirror configuration is created on AWS VMS.
steps:

  1. Create ECR registry and push kubernetes images
./dkp push bundle \
--bundle kubernetes-images-1.27.6-d2iq.1.tar \
--to-registry 999867407951.dkr.ecr.us-west-2.amazonaws.com/shalin-mirror \
--to-registry-username AWS \
--to-registry-password $(aws ecr get-login-password) \
--ecr-lifecycle-policy-file ecr-lifecycle-policy.json
  1. create aws creds secrets as per README file
  2. Create cluster using mirror variable
apiVersion: cluster.x-k8s.io/v1beta1
kind: Cluster
metadata:
  labels:
    cluster.x-k8s.io/provider: aws
  name: aws-quick-start
spec:
  clusterNetwork:
    pods:
      cidrBlocks:
        - 192.168.0.0/16
    serviceDomain: cluster.local
    services:
      cidrBlocks:
        - 10.128.0.0/12
  topology:
    class: aws-quick-start
    controlPlane:
      replicas: 1
    variables:
      - name: clusterConfig
        value:
          addons:
            cni:
              provider: calico
            cpi: {}
            csi:
              providers:
                - name: aws-ebs
            nfd: {}
          aws:
            region: us-west-2
          imageRegistries:
            - url: https://999867407951.dkr.ecr.us-west-2.amazonaws.com/shalin-mirror
              mirror: {}
    version: v1.27.5
    workers:
      machineDeployments:
        - class: default-worker
          name: md-0
          replicas: 1
---
apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
kind: AWSClusterStaticIdentity
metadata:
  labels:
    cluster.x-k8s.io/provider: aws
  name: aws-quick-start
spec:
  allowedNamespaces:
    list:
      - default
  secretRef: aws-quick-start-creds
  1. Checked that the kubeadmcontrolplane and kubeadmconfigtempnlate has following files
- content: |
    apiVersion: credentialprovider.d2iq.com/v1alpha1
    kind: DynamicCredentialProviderConfig
    mirror:
      endpoint: "999867407951.dkr.ecr.us-west-2.amazonaws.com/shalin-mirror"
      credentialsStrategy: "MirrorCredentialsOnly"
    credentialProviderPluginBinDir: /etc/kubernetes/image-credential-provider/
    credentialProviders:
      apiVersion: kubelet.config.k8s.io/v1beta1
      kind: CredentialProviderConfig
      providers:
      - name: ecr-credential-provider
        args:
        - get-credentials
        matchImages:
        - "999867407951.dkr.ecr.us-west-2.amazonaws.com/shalin-mirror"
        defaultCacheDuration: "0s"
        apiVersion: credentialprovider.kubelet.k8s.io/v1alpha1
  path: /etc/kubernetes/dynamic-credential-provider-config.yaml
  permissions: "0600"
- content: |
    [host."https://999867407951.dkr.ecr.us-west-2.amazonaws.com/shalin-mirror"]
      capabilities = ["pull", "resolve"]
  path: /etc/containerd/certs.d/_default/hosts.toml
  permissions: "0600"
  1. Checked on the VMs in AWS that above files are created

The cluster partially came up, CNI pods were not starting.
However I am unable to determine from logs of kubelet or containerd if mirror was indeed used to pull image and mirror credentials were used by dynamic credentials provider. I will sync up with the team and figure out how to confirm if images pulled from the mirror in networked environment.

Another alternative is to create cluster in airgap environment with ECR mirror. however it will require a lot more additional setup manually which we are planning to perform in konvoy e2e test for cluser class anyways.

@jimmidyson
Copy link
Member

What happens if multiple clusters are marked as mirror? I wonder if the config should look more like:

variables:
  - name: clusterConfig
    value:
      imageRegistries:
        credentials:
          - url: https://my-registry.io/
            secretRef:
              name: my-registry-credentials
        mirror: https://my-registry.io/

@jimmidyson
Copy link
Member

jimmidyson commented Jan 18, 2024

Please review and merge #293 and then rebase this PR.

@supershal
Copy link
Contributor Author

What happens if multiple clusters are marked as mirror? I wonder if the config should look more like:

variables:
  - name: clusterConfig
    value:
      imageRegistries:
        credentials:
          - url: https://my-registry.io/
            secretRef:
              name: my-registry-credentials
        mirror: https://my-registry.io/

We did considered this during our discussions.
konvoy currently supports only one mirror registry and credentials for that mirror registry. This can change in future.
Current implementation errors out if more than one registry is provided.

IMHO imageRegistries should be an array of imageRegistry with each registry can have one set of static credentials and each potentially can be used as a mirror.

variables:
   - name: clusterConfig
     value:
       imageRegistries:
           - url: https://registry-1.io/
              credentials:           
                  secretRef:
                     name: registry-1-credentials
              mirror: {}
           - url: https://registry-2.io/
              credentials:           
                  secretRef:
                     name: registry-2-credentials
              mirror: 
                 secretRef:
                     name: registry-2-CA-secret

We have a google doc with all suggested options for the schema. I will start a thread in slack to discuss it.

@supershal
Copy link
Contributor Author

Created stacked PR #296 to fix flakey unit tests. blocking this PR until its merged.

api/v1alpha1/clusterconfig_types.go Outdated Show resolved Hide resolved
api/v1alpha1/clusterconfig_types.go Outdated Show resolved Hide resolved
api/v1alpha1/clusterconfig_types.go Outdated Show resolved Hide resolved
api/v1alpha1/clusterconfig_types.go Show resolved Hide resolved
api/v1alpha1/clusterconfig_types.go Outdated Show resolved Hide resolved
docs/content/customization/generic/global-mirror.md Outdated Show resolved Hide resolved
docs/content/customization/generic/global-mirror.md Outdated Show resolved Hide resolved
docs/content/customization/generic/global-mirror.md Outdated Show resolved Hide resolved
docs/content/customization/generic/global-mirror.md Outdated Show resolved Hide resolved
docs/content/customization/generic/global-mirror.md Outdated Show resolved Hide resolved
api/v1alpha1/clusterconfig_types.go Outdated Show resolved Hide resolved
Copy link
Member

@jimmidyson jimmidyson left a comment

Choose a reason for hiding this comment

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

Pushed changes from comments, thanks for this PR @supershal!

@jimmidyson jimmidyson enabled auto-merge (squash) February 6, 2024 12:55
@jimmidyson jimmidyson merged commit f12d3d7 into main Feb 6, 2024
10 checks passed
@jimmidyson jimmidyson deleted the shalin/containerd-mirror branch February 6, 2024 12:58
@github-actions github-actions bot mentioned this pull request Feb 6, 2024
@supershal
Copy link
Contributor Author

Thank you @jimmidyson for the final changes.

dkoshkin pushed a commit that referenced this pull request Feb 8, 2024
🤖 I have created a release *beep* *boop*
---


## 0.3.0 (2024-02-07)

<!-- Release notes generated using configuration in .github/release.yaml
at main -->

## What's Changed
### Exciting New Features 🎉
* feat: starts additional sec groups by @faiq in
#252
* feat: add control-plane load balancer scheme patch by @dkoshkin in
#228
* feat: Pull in CAAPH APIs by @jimmidyson in
#282
* feat: Use latest dynamic credential provider and v1 kubelet API by
@jimmidyson in
#293
* feat: Add ClusterResourceSet strategy for CNI installation by
@jimmidyson in
#288
* feat: Use CAAPH to deploy Calico on workload clusters by @jimmidyson
in #283
* feat: containerd configuration for mirror registry by @supershal in
#292
* feat: introduce a Go module for /api by @dkoshkin in
#331
### Fixes 🔧
* fix: Stable EBS CSI manifests by @jimmidyson in
#270
* fix: Ensure registry credentials are namespace local to Cluster by
@jimmidyson in
#332
### Other Changes
* build: Upgrade devbox tools by @jimmidyson in
#271
* ci: Update release please configuration for v4 action by @jimmidyson
in #274
* build: Add release conventional commut type for release PRs by
@jimmidyson in
#276
* docs: Add intro page to user docs by @jimmidyson in
#280
* build: Use ko for building OCI image by @jimmidyson in
#281
* build: Add files for clusterctl compatibility by @jimmidyson in
#284
* build: local development in macOS(and Linux) arm64/amd64 using local
colima instance by @supershal in
#285
* build: Lint for missed errors in tests too by @jimmidyson in
#287
* build: Remove unused upx makefile stuff by @jimmidyson in
#291
* docs: Fix indentation of AWS secret example by @jimmidyson in
#294
* build: Add k8s 1.28 KinD for testing by default by @jimmidyson in
#295
* build: Add devbox update scheduled job by @jimmidyson in
#310
* build(main): Latest devbox update (2024-01-22) by @github-actions in
#315
* ci: Group k8s mod updates for dependabot by @jimmidyson in
#316
* build(main): Latest devbox update (2024-01-24) by
@d2iq-labs-actions-pr-bot in
#320
* build(main): Latest devbox update (2024-02-05) by
@d2iq-labs-actions-pr-bot in
#326
* docs: fix cluster name in README by @dkoshkin in
#330
* ci: Consistent bash defaults in workflows by @jimmidyson in
#336
* ci: Tag api module on release by @jimmidyson in
#335

## New Contributors
* @d2iq-labs-actions-pr-bot made their first contribution in
#320

**Full Changelog**:
v0.2.0...v0.3.0

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot added feature and removed feature labels Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Patch: Generic Containerd mirror support
4 participants