-
Notifications
You must be signed in to change notification settings - Fork 13
Pass VM POD name & namespace to shutdown-deferrer (and update images) #592
Conversation
In order to find the right DrainerConfig for node drainage status, shutdown-deferrer needs to know the POD name and namespace it's running in. This is passed via Downward API as environment parameter.
Because of node POD running with host network, bind to loopback address fails when using same port for all pods (when there's more than one tenant node on physical control plane machine). Use the same trick as for flannel health and calculate listen port from flannel VNI.
…/giantswarm/kvm-operator into pass-pod-info-to-shutdown-deferrer
Are e2e tests failing because of this change? How did manual testing go? Would be nice to have this info while reviewing. |
service/controller/v15/resource/deployment/master_deployment.go
Outdated
Show resolved
Hide resolved
r.logger.LogCtx(ctx, "level", "debug", "message", "keeping finalizers") | ||
finalizerskeptcontext.SetKept(ctx) | ||
|
||
return nil, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code here is nice.
Lifecycle: &apiv1.Lifecycle{ | ||
PreStop: &apiv1.Handler{ | ||
Exec: &apiv1.ExecAction{ | ||
Command: []string{"/pre-shutdown-hook", fmt.Sprintf("%s/v1/defer/", key.ShutdownDeferrerListenAddress(customObject))}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now I see the whole picture. In a hindsight I think this whole shutdown-deferer
repo could be put as another kvm-operator endpoint. We wouldn't have to maintain separate docker image.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original idea was to have reusable container that can be used for this purpose, but now that you say it, it is a bit dependent on the use case here so could make sense.
Co-Authored-By: tuommaki <1947505+tuommaki@users.noreply.github.com>
Co-Authored-By: tuommaki <1947505+tuommaki@users.noreply.github.com>
Co-Authored-By: tuommaki <1947505+tuommaki@users.noreply.github.com>
Co-Authored-By: tuommaki <1947505+tuommaki@users.noreply.github.com>
Co-Authored-By: tuommaki <1947505+tuommaki@users.noreply.github.com>
Co-Authored-By: tuommaki <1947505+tuommaki@users.noreply.github.com>
Co-Authored-By: tuommaki <1947505+tuommaki@users.noreply.github.com>
Co-Authored-By: tuommaki <1947505+tuommaki@users.noreply.github.com>
Co-Authored-By: tuommaki <1947505+tuommaki@users.noreply.github.com>
Co-Authored-By: tuommaki <1947505+tuommaki@users.noreply.github.com>
…/giantswarm/kvm-operator into pass-pod-info-to-shutdown-deferrer
Unsure about e2e but I'm pretty sure it's about environments. We agreed with @teemow that he'll test this manually in order to proceed with upcoming release. |
Oh, but approvals are not there. Waiting for them :) PTAL 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine with iterating here. Since you tested this manually let's go. KVM e2e is not really a usable thing yet IMO.
@@ -370,6 +377,18 @@ func ServiceAccountName(customObject v1alpha1.KVMConfig) string { | |||
return ClusterID(customObject) | |||
} | |||
|
|||
func ShutdownDeferrerListenPort(customObject v1alpha1.KVMConfig) int { | |||
return int(shutdownDeferrerPortBase + customObject.Spec.KVM.Network.Flannel.VNI) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return int(shutdownDeferrerPortBase + customObject.Spec.KVM.Network.Flannel.VNI) | |
// per tenant cluster we run one vm per host. the pod of the vm uses the host network. so this makes sure the port of the shutdown deferrer is unique per tenant vm. | |
return int(shutdownDeferrerPortBase + customObject.Spec.KVM.Network.Flannel.VNI) |
In order to find the right DrainerConfig for node drainage status,
shutdown-deferrer needs to know the POD name and namespace it's running
in. This is passed via Downward API as environment parameter.
Also prevent Endpoint to be deleted before corresponding node is shut down.