Skip to content

Make the name of the webhook certs more metallb specific #2244

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
Merged
merged 1 commit into from
Feb 15, 2024
Merged

Make the name of the webhook certs more metallb specific #2244

merged 1 commit into from
Feb 15, 2024

Conversation

cyclinder
Copy link
Contributor

@cyclinder cyclinder commented Jan 15, 2024 •
edited
Loading

Rename webhook_service/webhook_cert name to avoid name conflicts with other resources in the cluster.

Fixes #2174

@cyclinder cyclinder force-pushed the webhook_cert branch 3 times, most recently from f66a7a6 to 9224d2e Compare January 15, 2024 14:32
@fedepaol
Copy link
Member

@cyclinder mind checking the CI failures? I guess something went wrong.
Thanks!

@cyclinder
Copy link
Contributor Author

cyclinder commented Jan 30, 2024 •
edited
Loading

sure, let me check these failures

@cyclinder cyclinder force-pushed the webhook_cert branch 3 times, most recently from dbe0362 to a6a670c Compare January 31, 2024 10:34
@cyclinder
Copy link
Contributor Author

Hi @fedepaol, If we update the names of webhook_service/webhook_cert directly, this may be an incompatible change, which leads to failures with metallb-operator (the old manifests use the new docker-image, and the correct name is not specified in the controller via go flags). I think there are two ways:

  1. Update metallb-operator to make it compatible with this change
  2. Close this PR, webhook_service/webhook_cert are already configured.

I think 2 is enough, but I prefer 1, I like the default value to be metallb related.

@jonasbadstuebner
Copy link
Contributor

Can you fix https://github.com/metallb/metallb/blob/main/charts/metallb/charts/crds/templates/crds.yaml#L260 before everything else?
Because we see a lot of

cacher (bgppeers.metallb.io): unexpected ListAndWatch error: failed to list metallb.io/v1beta1, Kind=BGPPeer: conversion webhook for metallb.io/v1beta2, Kind=BGPPeer failed: Post "https://webhook-service.metallb.svc:443/convert?timeout=30s": service "webhook-service" not found; reinitializing...

in out api-server.

It was introduced here 5339a9a I think. Since the service was renamed from metallb-webhook-service to webhook-service.

@fedepaol
Copy link
Member

@cyclinder mind fixing the conflicts?

@cyclinder cyclinder force-pushed the webhook_cert branch 2 times, most recently from 6d1fb25 to eaef2a7 Compare February 15, 2024 03:00
@cyclinder cyclinder changed the title controller: rename webhook_service/webhook_cert name make the names of webhook service and cert configurable Feb 15, 2024
@fedepaol
Copy link
Member

Just a nit on the commit / PR title: here we are not making the name configurable, but fixing it to a metallb-only value.

@fedepaol fedepaol changed the title make the names of webhook service and cert configurable Make the name of the webhook certs more metallb specific Feb 15, 2024
@cyclinder @fedepaol
Make the name of the webhook certs more metallb specific
774bbf6 
make the names of the webhook and cert metallb specific to
avoid name conflicts with other resources in the cluster.

Signed-off-by: cyclinder <qifeng.guo@daocloud.io>
@fedepaol
Copy link
Member

LGTM

@fedepaol fedepaol enabled auto-merge February 15, 2024 15:58
@fedepaol fedepaol added this pull request to the merge queue Feb 15, 2024
Merged via the queue into metallb:main with commit cd0b6fe Feb 15, 2024
@fedepaol fedepaol mentioned this pull request Mar 22, 2024
Closed
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fedepaol fedepaol Awaiting requested review from fedepaol fedepaol is a code owner

@russellb russellb Awaiting requested review from russellb

@gclawes gclawes Awaiting requested review from gclawes

@oribon oribon Awaiting requested review from oribon oribon is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

webhook_service/webhook_cert inconfigurable
3 participants
@cyclinder @fedepaol @jonasbadstuebner