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

Propagate registry mirror config for Nutanix provider #5236

Merged
merged 5 commits into from
Mar 15, 2023

Conversation

thunderboltsid
Copy link
Contributor

@thunderboltsid thunderboltsid commented Mar 13, 2023

To support EKS-Anywhere on Nutanix in air-gapped environments, we need to honor the
local registry mirror specified in the cluster config and propagate it forward.

How has this been tested?
Followed https://anywhere.eks.amazonaws.com/docs/reference/clusterspec/optional/registrymirror/ to set up a
harbor registry with necessary projects and importing the images to the harbor registry.

$ bin/eksctl-anywhere import images -i eks-anywhere-images.tar --bundles eks-anywhere-downloads/bundle-release.yaml --registry ${REGISTRY_ENDPOINT}

Add the registryMirrorConfiguration property to the cluster spec pointing to the new registry.

apiVersion: anywhere.eks.amazonaws.com/v1alpha1
kind: Cluster
metadata:
  name: eksa-sid
spec:
  ...
  registryMirrorConfiguration:
    endpoint: registry.ntnxsherlock.com
    ociNamespaces:
      - registry: "public.ecr.aws/l0g8r8j6"
        namespace: "eks-anywhere"
    authenticate: true
    insecureSkipVerify: true

Using a bundle-release.yaml with an image of the controller built from the changes in this PR as bundles override, create a cluster.

$ bin/eksctl-anywhere create cluster -f ./${CLUSTER_NAME}.yaml --bundles-override ./bundle-release.yaml
Logging in to docker registry registry.ntnxsherlock.com
Logging in to docker registry registry.ntnxsherlock.com
Logging in to docker registry registry.ntnxsherlock.com
Logging in to docker registry registry.ntnxsherlock.com
Logging in to docker registry registry.ntnxsherlock.com
Logging in to docker registry registry.ntnxsherlock.com
Logging in to docker registry registry.ntnxsherlock.com
Logging in to docker registry registry.ntnxsherlock.com
Logging in to docker registry registry.ntnxsherlock.com
Logging in to docker registry registry.ntnxsherlock.com
Logging in to docker registry registry.ntnxsherlock.com
Logging in to docker registry registry.ntnxsherlock.com
Logging in to docker registry registry.ntnxsherlock.com
Logging in to docker registry registry.ntnxsherlock.com
Logging in to docker registry registry.ntnxsherlock.com
Logging in to docker registry registry.ntnxsherlock.com
Logging in to docker registry registry.ntnxsherlock.com
Logging in to docker registry registry.ntnxsherlock.com
Performing setup and validations
✅ Nutanix Provider setup is valid
✅ Validate certificate for registry mirror
✅ Validate authentication for git provider
Creating new bootstrap cluster
Provider specific pre-capi-install-setup on bootstrap cluster
Installing cluster-api providers on bootstrap cluster
Provider specific post-setup
Creating new workload cluster
Installing networking on workload cluster
Logging in to helm registry	{"registry": "registry.ntnxsherlock.com:443"}
Creating EKS-A namespace
Installing cluster-api providers on workload cluster
Installing EKS-A secrets on workload cluster
Installing resources on management cluster
Moving cluster management from bootstrap to workload cluster
Installing EKS-A custom components (CRD and controller) on workload cluster
Installing EKS-D components on workload cluster
Creating EKS-A CRDs instances on workload cluster
Installing GitOps Toolkit on workload cluster
GitOps field not specified, bootstrap flux skipped
Writing cluster config file
Deleting bootstrap cluster
🎉 Cluster created!

@eks-distro-bot
Copy link
Collaborator

Hi @thunderboltsid. Thanks for your PR.

I'm waiting for a aws member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@eks-distro-bot eks-distro-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Mar 13, 2023
@eks-distro-bot eks-distro-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 13, 2023
@cxbrowne1207
Copy link
Member

cxbrowne1207 commented Mar 13, 2023

Can support for the InsecureSkipVerify flag in RegistryMirrorConfiguration be included as well?

Here is the related issue describing the efforts to propagate it across all providers. You can also reference this PR

@eks-distro-bot eks-distro-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 13, 2023
@thunderboltsid thunderboltsid force-pushed the ntnx-local-registry branch 2 times, most recently from 8e2103f to 0c3cc06 Compare March 13, 2023 20:08
@codecov
Copy link

codecov bot commented Mar 13, 2023

Codecov Report

Merging #5236 (59966a2) into main (c8616e7) will increase coverage by 0.03%.
The diff coverage is 100.00%.

❗ Current head 59966a2 differs from pull request most recent head bbd5bd0. Consider uploading reports for the commit bbd5bd0 to get more accurate results

@@            Coverage Diff             @@
##             main    #5236      +/-   ##
==========================================
+ Coverage   72.18%   72.21%   +0.03%     
==========================================
  Files         435      435              
  Lines       35469    35475       +6     
==========================================
+ Hits        25602    25620      +18     
+ Misses       8326     8315      -11     
+ Partials     1541     1540       -1     
Impacted Files Coverage Δ
pkg/api/v1alpha1/cluster.go 76.38% <100.00%> (+0.03%) ⬆️
pkg/config/registry.go 70.00% <100.00%> (+70.00%) ⬆️
pkg/providers/nutanix/template.go 92.89% <100.00%> (+2.15%) ⬆️

... and 9 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@thunderboltsid
Copy link
Contributor Author

Can support for the InsecureSkipVerify flag in RegistryMirrorConfiguration be included as well?

Here is the related issue describing the efforts to propagate it across all providers. You can also reference this PR

Sure.

@eks-distro-bot eks-distro-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 14, 2023
@thunderboltsid
Copy link
Contributor Author

thunderboltsid commented Mar 14, 2023

@cxbrowne1207 Updated the PR with support for propagating of skipping TLS verification in the templates. Can you please take a look?

@thunderboltsid thunderboltsid changed the title [WIP] Propagate registry mirror config for Nutanix provider Propagate registry mirror config for Nutanix provider Mar 14, 2023
@thunderboltsid
Copy link
Contributor Author

/assign @abhinavmpandey08

@abhinavmpandey08
Copy link
Member

/ok-to-test

@thunderboltsid thunderboltsid force-pushed the ntnx-local-registry branch 2 times, most recently from d7d5359 to 037d3d0 Compare March 14, 2023 12:50
Copy link
Member

@cxbrowne1207 cxbrowne1207 left a comment

Choose a reason for hiding this comment

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

/lgtm

@eks-distro-bot eks-distro-bot added lgtm and removed lgtm labels Mar 14, 2023
@thunderboltsid thunderboltsid force-pushed the ntnx-local-registry branch 2 times, most recently from 1796a41 to ef09260 Compare March 15, 2023 00:29
@ndeksa ndeksa requested a review from d8660091 March 15, 2023 01:29
@thunderboltsid thunderboltsid requested review from cxbrowne1207 and removed request for d8660091 March 15, 2023 14:11
@thunderboltsid
Copy link
Contributor Author

/assign @d8660091

@thunderboltsid
Copy link
Contributor Author

@cxbrowne1207 had to update the PR to add some more unit tests to maintain coverage numbers. Can you PTAL?

@cxbrowne1207
Copy link
Member

cxbrowne1207 commented Mar 15, 2023

/lgtm

Looks solid!

@@ -775,3 +776,41 @@ func TestNutanixKubernetes126OIDC(t *testing.T) {
)
runOIDCFlow(test)
}

// Registry Mirror Tests
func TestNutanixKubernetes124UbuntuRegistryMirrorAndCert(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

do you mind moving the e2e to a separate PR? We will need to setup some infra for this which we don't have currently.

@thunderboltsid thunderboltsid requested review from abhinavmpandey08 and removed request for cxbrowne1207 March 15, 2023 20:31
Copy link
Member

@abhinavmpandey08 abhinavmpandey08 left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm
/woof

@eks-distro-bot
Copy link
Collaborator

@abhinavmpandey08: dog image

In response to this:

/approve
/lgtm
/woof

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@eks-distro-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavmpandey08

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@eks-distro-bot eks-distro-bot merged commit f2dc386 into aws:main Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm ok-to-test size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants