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

Upgrade CoreDNS-Operator to v2 #50

Merged
merged 2 commits into from
May 26, 2020

Conversation

rajansandeep
Copy link
Contributor

@rajansandeep rajansandeep commented Mar 23, 2020

The CoreDNS operator is built using Kubebuilder v2.3 and a bug fix which fixes the plugin addon template.

Edit: Since then v2.3.1 has been released which includes the bug fix

This replaces the existing v1 operator
(If the changes here are confusing due to file overlap, or we prefer to keep v1, I can make these changes to a different directory)

I have also updated the CoreDNS version to 1.6.7 (Current latest)

TODO:

  • Boilerplate
  • Operator runs outside cluster
  • Operator runs in-cluster
  • Support CoreDNS upgrade path
  • Support Kustomize for config hash
  • Golden-tests

Fixes: #24

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 23, 2020
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Mar 23, 2020
@dholbach
Copy link
Member

/cc @johnsonj
/uncc

@k8s-ci-robot k8s-ci-robot requested review from johnsonj and removed request for dholbach March 24, 2020 07:42
@somtochiama somtochiama mentioned this pull request Apr 3, 2020
4 tasks
@rajansandeep
Copy link
Contributor Author

rajansandeep commented Apr 29, 2020

The Kustomize support complies using patch from kubernetes-sigs/kubebuilder-declarative-pattern#91

Edit: Both kubernetes-sigs/kubebuilder-declarative-pattern#91 and kubernetes-sigs/kubebuilder-declarative-pattern#94 are merged and included in "master"

@rajansandeep rajansandeep changed the title WIP: Upgrade CoreDNS-Operator to v2 Upgrade CoreDNS-Operator to v2 Apr 29, 2020
@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 Apr 29, 2020
@rajansandeep rajansandeep force-pushed the coredns-operator-v2 branch from bb6c673 to 286f134 Compare May 13, 2020 17:56
…r version 2

some cleanup, add 1.3.1 coredns, make operator run in-cluster

add basic golden test

add more golden test and made minor nits

support kustomize, add coredns migration logic and add some tests

update golden-test result and update go mod
@rajansandeep rajansandeep force-pushed the coredns-operator-v2 branch from 286f134 to 8e80324 Compare May 13, 2020 18:01
@rajansandeep
Copy link
Contributor Author

/assign @johnsonj
PTAL when possible, thank you!

@johnsonj
Copy link
Contributor

wohoo! will do- thank you @rajansandeep !

Copy link
Contributor

@johnsonj johnsonj left a comment

Choose a reason for hiding this comment

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

@rajansandeep - This looks fantastic. The migration implementation is straightforward and clear. This is exactly the use-case that makes the operator shine. I appreciate your digging and fixes to get kustomize in kubebuilder-declarative-pattern working! The testing looks great as well.

I'd suggest in this change (or another) to beef up the README since this thing is no toy now!

coredns/Makefile Outdated
@@ -1,66 +1,85 @@

# Image URL to use all building/pushing image targets
IMG ?= ${USER}/coredns-operator:latest
IMG ?= rajansandeep/coredns-operator:latest
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: revert back to ${USER}

@rajansandeep
Copy link
Contributor Author

I'd suggest in this change (or another) to beef up the README since this thing is no toy now!

@johnsonj, Definitely! I think the README should be focusing on "How to use/install" the operator than "How to create".

I'll make those changes in a separate PR.

@johnsonj
Copy link
Contributor

/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 May 26, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johnsonj, rajansandeep

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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 26, 2020
@k8s-ci-robot k8s-ci-robot merged commit 2e70e26 into kubernetes-sigs:master May 26, 2020
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
Development

Successfully merging this pull request may close these issues.

Upgrade coredns to kubebuilder 2
4 participants