optimize the webhook manager #3588
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
volcano-sh-bot
added
the
size/S
|
Hi, please make CI happy. |
|
If we remove the existing flags directly, users who upgrade from an old verison meet errors about the flag, so it's not recommended to remove them: ) |
|
I think these flags are used to let kube-apiserver know the address of wenhook and access it so we'd better keep them. |
Vacant2333
force-pushed
the
optimize-webhook-manager-codes
branch
from
e8ebab2
to
5ea21fd
Compare
volcano-sh-bot
added
size/XS
Vacant2333
force-pushed
the
optimize-webhook-manager-codes
branch
from
5ea21fd
to
363be18
Compare
| @@ -101,7 +102,7 @@ func Run(config *options.Config) error { | |||
| } | |||
| go func() { | |||
| err = server.ListenAndServeTLS("", "") | |||
| if err != nil && err != http.ErrServerClosed { | |||
| if err != nil && !errors.Is(err, http.ErrServerClosed) { | |||
There was a problem hiding this comment.
IMO, this change does not seem to change any logic. Can you explain the benefits of such a change?
| @@ -58,6 +58,8 @@ func ForEachAdmission(config *options.Config, handler func(*AdmissionService) er | |||
| if err := handler(service); err != nil { | |||
| return err | |||
| } | |||
| } else { | |||
| return fmt.Errorf("enabled admission %s not found on the admission registered map", admission) | |||
There was a problem hiding this comment.
Why is there an error if admission is not found?
ForEachAdmissionwill not tell u if a webhook not register to the manager, i find this problem when i use this manager in my project, and its hard to find