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.

Sign up for GitHub

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

Jump to bottom

🌱 CAPIM: Enable update for coreDNS and kube-proxy #8899

Merged
merged 1 commit into from
Jun 23, 2023
Merged

🌱 CAPIM: Enable update for coreDNS and kube-proxy #8899

merged 1 commit into from
Jun 23, 2023

Conversation

killianmuldoon
Copy link
Contributor

Enable the CAPIM to respond to calls to update coreDNS and kube-proxy.

Part of #8814

@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 Jun 21, 2023
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 21, 2023
killianmuldoon
killianmuldoon commented Jun 21, 2023
View reviewed changes
Copy link
Contributor Author

@killianmuldoon killianmuldoon left a comment

Choose a reason for hiding this comment

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

At present this just enables the apiServers to respond to the calls for the daemonsets. This results in a non-error on reconciling these functions.

Still have to take another look at what this would actually look like through an upgrade flow.

@killianmuldoon killianmuldoon force-pushed the pr-in-memory-update-coredns-kubeproxy branch from 77a7d55 to 18bc71b Compare June 22, 2023 14:18
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 22, 2023
killianmuldoon
killianmuldoon commented Jun 22, 2023
View reviewed changes
Copy link
Contributor Author

@killianmuldoon killianmuldoon left a comment

Choose a reason for hiding this comment

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

This PR only covers the base case for these functions now.

Not covered:

  • handling for clusterRole and v1.22 migration (not sure how long we have to keep this code around, but there's a lot of complexity around version checking and probably not worth it here.
  • handling for upgrades of coreDNS - will be done under the upgrades work.

@killianmuldoon killianmuldoon force-pushed the pr-in-memory-update-coredns-kubeproxy branch from 18bc71b to dcb952e Compare June 22, 2023 14:51
@killianmuldoon killianmuldoon changed the title [WIP] 🌱 CAPIM: Enable update for coreDNS and kube-proxy 🌱 CAPIM: Enable update for coreDNS and kube-proxy Jun 22, 2023
@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 Jun 22, 2023
@killianmuldoon killianmuldoon force-pushed the pr-in-memory-update-coredns-kubeproxy branch from dcb952e to fc1c954 Compare June 22, 2023 14:52
fabriziopandini
fabriziopandini reviewed Jun 22, 2023
View reviewed changes
Copy link
Member

@fabriziopandini fabriziopandini left a comment

Choose a reason for hiding this comment

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

two nits, otherwise lgtm

test/infrastructure/inmemory/internal/controllers/inmemorymachine_controller.go Outdated Show resolved Hide resolved
test/infrastructure/inmemory/internal/controllers/inmemorymachine_controller.go Outdated Show resolved Hide resolved
test/infrastructure/inmemory/internal/controllers/inmemorymachine_controller.go Outdated Show resolved Hide resolved
test/infrastructure/inmemory/internal/controllers/inmemorymachine_controller.go Outdated Show resolved Hide resolved
@killianmuldoon killianmuldoon force-pushed the pr-in-memory-update-coredns-kubeproxy branch from fc1c954 to 5860700 Compare June 22, 2023 16:10
@fabriziopandini
Copy link
Member

/lgtm

cc @sbueringer

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 22, 2023
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 2785c18e769f14a3e42b5bcb9376e8151f217214

sbueringer
sbueringer reviewed Jun 23, 2023
View reviewed changes
Copy link
Member

@sbueringer sbueringer left a comment

Choose a reason for hiding this comment

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

Overall looks good

test/infrastructure/inmemory/internal/controllers/inmemorymachine_controller.go Outdated Show resolved Hide resolved
test/infrastructure/inmemory/internal/controllers/inmemorymachine_controller.go Outdated Show resolved Hide resolved
test/infrastructure/inmemory/templates/clusterclass-in-memory-quick-start.yaml Show resolved Hide resolved
@killianmuldoon killianmuldoon force-pushed the pr-in-memory-update-coredns-kubeproxy branch from 5860700 to d9203ab Compare June 23, 2023 10:24
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 23, 2023
@killianmuldoon killianmuldoon force-pushed the pr-in-memory-update-coredns-kubeproxy branch from d9203ab to 6b5e7cf Compare June 23, 2023 10:40
@killianmuldoon killianmuldoon force-pushed the pr-in-memory-update-coredns-kubeproxy branch 2 times, most recently from b3eed05 to 8b13350 Compare June 23, 2023 10:50
@killianmuldoon
CAPIM: Enable update for coreDNS and kube-proxy
c85ce8e 
Signed-off-by: killianmuldoon <kmuldoon@vmware.com>
@killianmuldoon killianmuldoon force-pushed the pr-in-memory-update-coredns-kubeproxy branch from 8b13350 to c85ce8e Compare June 23, 2023 10:52
@sbueringer
Copy link
Member

/lgtm
/approve

/hold
not sure if you want to wait for other reviews

@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 Jun 23, 2023
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 23, 2023
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 2b1a0055a4d28e15c0ec436dc00cf1195aa98e59

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbueringer

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 Jun 23, 2023
@killianmuldoon
Copy link
Contributor Author

/hold cancel

@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 Jun 23, 2023
@k8s-ci-robot k8s-ci-robot merged commit ff78516 into kubernetes-sigs:main Jun 23, 2023
10 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v1.5 milestone Jun 23, 2023
@sbueringer sbueringer mentioned this pull request Jun 23, 2023
Open
27 tasks
@killianmuldoon
Copy link
Contributor Author

/area provider/infrastructure-in-memory

(doesn't exist yet)

@k8s-ci-robot k8s-ci-robot added the area/provider/infrastructure-in-memory Issues or PRs related to the in-memory infrastructure provider label Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sbueringer sbueringer sbueringer left review comments

@richardcase richardcase Awaiting requested review from richardcase

@stmcginnis stmcginnis Awaiting requested review from stmcginnis

@fabriziopandini fabriziopandini Awaiting requested review from fabriziopandini

Assignees

@sbueringer sbueringer

@fabriziopandini fabriziopandini

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/provider/infrastructure-in-memory Issues or PRs related to the in-memory infrastructure provider 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/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.5
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@killianmuldoon @fabriziopandini @k8s-ci-robot @sbueringer