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

Migrate to CRDs, Kubebuilder and Kustomize #126

Merged
merged 29 commits into from Feb 6, 2019

Conversation

xmudrii
Copy link
Member

@xmudrii xmudrii commented Jan 1, 2019

What this PR does / why we need it:

The PR is reviewable.

This PR refactors the Cluster-API Provider for DigitalOcean to use the latest upstream Cluster-API implementation, which includes changes made as of October 5th with kubernetes-sigs/cluster-api#494 being the most important one.

The most important changes and refactors in this PR include:

  • Cluster-API Provider for DigitalOcean has switched from the Aggregated API Server Cluster-API implementation to the CRDs based implementation based on Kubebuilder (Migrate to kubebuilder cluster-api#494). 🎉
  • Manifests for cluster creation are now being generated with the generation Bash script and Kustomize.
  • Cluster and Machine Controllers are removed and replaced by the Manager binary (removal done in Remove Cluster and Machine Controller #125).
  • Cluster and Machine Actuator interfaces are refactored, so it's possible to easily utilize and call them from the Manager.
  • [Breaking change] Machine objects are modified to use the providerSpec field instead of the providerConfig field.
  • clusterctl is refactored to be compatible with the latest changes.
  • Makefile is refactored to include targets needed by Kubebuilder. Already existing targets may be changed, removed or renamed.

Requirements changes:

  • Kustomize is now required in order to generate manifests for cluster creation.
  • Due to security issues it is recommended to use Minikube v0.30+.

Merge requirements:
Before merging the PR the following points must be satisfied:

  • The PR Remove Cluster and Machine Controller #125 is merged and this PR is rebased against it.
  • The Manager binary images are built, pushed, and manifests are updated to use the correct image.
    • There are four options: continue using Kubermatic Quay.io account, create account on Docker Hub just for this project, push under personal Docker Hub account, or try to get GCR Bucket for the project.
    • Makefile needs to be updated to use the correct image.
  • The CI pipeline and the Makefile are fixed. As a result, tests are passing.
  • The documentation is updated to reflect the latest changes.
  • The kubeadm token generation bug is fixed.
  • Make sure there are no left TODO markers.
  • It is verified that Cluster-API Provider can successfully create cluster from zero using clusterctl.

Follow-up requirements:

  • Conformance tests are passing on cluster created by this provider.
  • Example manifests are updated to build Kubernetes 1.13+ cluster instead of 1.11 cluster.
  • Add-ons are updated to their latest versions.
  • The Failed to create ssh public key, the key already exists. error message is updated, so it's not confusing for users.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):

Fixes #83
Closes #78
Closes #94

Special notes for your reviewer:

The commits in this PR are split by the component, i.e. for each component changed, introduced, or removed, there is specific commit. Hopefully this will make PR easier to review.

Documentation:

Documentation is updated to match all changes made in this PR.

Release note:

Cluster-API Provider for DigitalOcean now uses CRDs/Kubebuilder based Cluster-API implementation. Kustomize is being used for scripts generation. The new Manager binary is introduced instead of Cluster and Machine Controllers, which are removed. Machine objects now uses the ProviderSpec field instead of ProviderConfig.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 1, 2019
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 1, 2019
@xmudrii xmudrii changed the title [WIP] Migrate to CRDs, Kubebuilder and Kustomize Migrate to CRDs, Kubebuilder and Kustomize Jan 3, 2019
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 3, 2019
@xmudrii xmudrii changed the title Migrate to CRDs, Kubebuilder and Kustomize 🎉 Migrate to CRDs, Kubebuilder and Kustomize 🎉 Jan 3, 2019
@xmudrii xmudrii changed the title 🎉 Migrate to CRDs, Kubebuilder and Kustomize 🎉 Migrate to CRDs, Kubebuilder and Kustomize Jan 3, 2019
@xmudrii
Copy link
Member Author

xmudrii commented Jan 3, 2019

/hold
until merge requirements are not satisfied

/unassign @andrewsykim
/assign @nikhita @alvaroaleman
/this-is-fine

@k8s-ci-robot
Copy link

@xmudrii: dog image

In response to this:

/hold
until merge requirements are not satisfied

/unassign @andrewsykim
/assign @nikhita @alvaroaleman
/this-is-fine

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.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 3, 2019
This was referenced Jan 3, 2019
@xmudrii
Copy link
Member Author

xmudrii commented Jan 8, 2019

The PR is finally done! 🎉
I've tried my best to split this across several commits, so it's easier to review it. There are many boilerplate changes that may be skipped.

The Manager binary is pushed as quay.io/kubermatic/cluster-api-do-controller with the v1.0.0-alpha.1 tag.

Due to many breaking changes made in this PR, I propose v1.0.0 as the next version.

I've tested and confirmed that this provider still works as expected with all the latest changes.

Tests were performed using the following clusterctl command:

go run main.go create cluster \
    --provider digitalocean \
    --vm-driver kvm2 \
    -c ./examples/digitalocean/out/cluster.yaml \
    -m ./examples/digitalocean/out/machines.yaml \
    -p ./examples/digitalocean/out/provider-components.yaml \
    -a ./examples/digitalocean/out/addons.yaml \
    --minikube="kubernetes-version=v1.12.4"

which returned the following output:

I0108 18:41:33.212579   29437 createbootstrapcluster.go:28] Creating bootstrap cluster
I0108 18:44:22.060381   29437 clusterdeployer.go:95] Applying Cluster API stack to bootstrap cluster
I0108 18:44:22.060404   29437 applyclusterapicomponents.go:27] Applying Cluster API Provider Components
I0108 18:44:31.853925   29437 clusterdeployer.go:100] Provisioning target cluster via bootstrap cluster
I0108 18:44:31.947468   29437 applycluster.go:37] Creating cluster object test-1 in namespace "kube-system"
I0108 18:44:31.999940   29437 clusterdeployer.go:109] Creating master  in namespace "kube-system"
I0108 18:44:32.024837   29437 applymachines.go:37] Creating machines in namespace "kube-system"
I0108 18:45:32.145817   29437 clusterdeployer.go:114] Updating bootstrap cluster object for cluster test-1 in namespace "kube-system" with master () endpoint
I0108 18:45:32.921484   29437 clusterdeployer.go:119] Creating target cluster
I0108 18:47:49.189082   29437 applyaddons.go:25] Applying Addons
I0108 18:48:20.035686   29437 clusterdeployer.go:132] Applying Cluster API stack to target cluster
I0108 18:48:20.035721   29437 clusterdeployer.go:260] Applying Cluster API Provider Components
I0108 18:48:22.248006   29437 clusterdeployer.go:265] Pivoting Cluster API objects from bootstrap to target cluster.
I0108 18:48:22.491771   29437 clusterdeployer.go:319] Moved Cluster 'test-1'
I0108 18:48:22.603444   29437 clusterdeployer.go:359] Moved Machine 'digitalocean-fra1-master-w6m7w'
I0108 18:48:22.603477   29437 clusterdeployer.go:137] Saving provider components to the target cluster
I0108 18:48:22.795634   29437 clusterdeployer.go:150] Updating target cluster object with master () endpoint
I0108 18:48:23.604809   29437 clusterdeployer.go:155] Creating node machines in target cluster.
I0108 18:48:23.647451   29437 applymachines.go:37] Creating machines in namespace "kube-system"
I0108 18:49:23.751715   29437 clusterdeployer.go:160] Done provisioning cluster. You can now access your cluster with kubectl --kubeconfig kubeconfig
I0108 18:49:23.751848   29437 createbootstrapcluster.go:37] Cleaning up bootstrap cluster.

/hold cancel
/meow

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 8, 2019
@k8s-ci-robot
Copy link

@xmudrii: cat image

In response to this:

The PR is finally done! 🎉
I've tried my best to split this across several commits, so it's easier to review it. There are many boilerplate changes that may be skipped.

The Manager binary is pushed as quay.io/kubermatic/cluster-api-do-controller with the v1.0.0-alpha.1 tag.

Due to many breaking changes made in this PR, I propose v1.0.0 as the next version.

I've tested and confirmed that this provider still works as expected with all the latest changes.

Tests were performed using the following clusterctl command:

go run main.go create cluster \
   --provider digitalocean \
   --vm-driver kvm2 \
   -c ./examples/digitalocean/out/cluster.yaml \
   -m ./examples/digitalocean/out/machines.yaml \
   -p ./examples/digitalocean/out/provider-components.yaml \
   -a ./examples/digitalocean/out/addons.yaml \
   --minikube="kubernetes-version=v1.12.4"

which returned the following output:

I0108 18:41:33.212579   29437 createbootstrapcluster.go:28] Creating bootstrap cluster
I0108 18:44:22.060381   29437 clusterdeployer.go:95] Applying Cluster API stack to bootstrap cluster
I0108 18:44:22.060404   29437 applyclusterapicomponents.go:27] Applying Cluster API Provider Components
I0108 18:44:31.853925   29437 clusterdeployer.go:100] Provisioning target cluster via bootstrap cluster
I0108 18:44:31.947468   29437 applycluster.go:37] Creating cluster object test-1 in namespace "kube-system"
I0108 18:44:31.999940   29437 clusterdeployer.go:109] Creating master  in namespace "kube-system"
I0108 18:44:32.024837   29437 applymachines.go:37] Creating machines in namespace "kube-system"
I0108 18:45:32.145817   29437 clusterdeployer.go:114] Updating bootstrap cluster object for cluster test-1 in namespace "kube-system" with master () endpoint
I0108 18:45:32.921484   29437 clusterdeployer.go:119] Creating target cluster
I0108 18:47:49.189082   29437 applyaddons.go:25] Applying Addons
I0108 18:48:20.035686   29437 clusterdeployer.go:132] Applying Cluster API stack to target cluster
I0108 18:48:20.035721   29437 clusterdeployer.go:260] Applying Cluster API Provider Components
I0108 18:48:22.248006   29437 clusterdeployer.go:265] Pivoting Cluster API objects from bootstrap to target cluster.
I0108 18:48:22.491771   29437 clusterdeployer.go:319] Moved Cluster 'test-1'
I0108 18:48:22.603444   29437 clusterdeployer.go:359] Moved Machine 'digitalocean-fra1-master-w6m7w'
I0108 18:48:22.603477   29437 clusterdeployer.go:137] Saving provider components to the target cluster
I0108 18:48:22.795634   29437 clusterdeployer.go:150] Updating target cluster object with master () endpoint
I0108 18:48:23.604809   29437 clusterdeployer.go:155] Creating node machines in target cluster.
I0108 18:48:23.647451   29437 applymachines.go:37] Creating machines in namespace "kube-system"
I0108 18:49:23.751715   29437 clusterdeployer.go:160] Done provisioning cluster. You can now access your cluster with kubectl --kubeconfig kubeconfig
I0108 18:49:23.751848   29437 createbootstrapcluster.go:37] Cleaning up bootstrap cluster.

/hold cancel
/meow

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.

Copy link
Member

@nikhita nikhita left a comment

Choose a reason for hiding this comment

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

Overall lgtm.

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 6, 2019
@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nikhita, xmudrii

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

@k8s-ci-robot k8s-ci-robot merged commit a5b4cce into kubernetes-sigs:master Feb 6, 2019
@xmudrii xmudrii deleted the crds branch March 17, 2019 16:47
@nikhita nikhita mentioned this pull request Oct 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
4 participants