Kill etcd process exactly before taking a final snapshot

[plkokanov]

What this PR does / why we need it:
This PR slightly changes when the etcd process is killed if the owner check fails so that the connection from the kube-apiserver to the etcd is broken.
Previously a corner case could happen which would leave etcd running and therefore the shoot's control plane would not get properly shut down in the source seed during control plane migration leading to split brain scenario:
This corner case can happen in the following scneario (but is probably not limited only to it)

1. If the backup bucket is not reachable for any reason backup-restore will fail to create a snapshot
2. backup-restore constantly loops trying to start the snapshotter
3. Control plane migration is triggered and eventually owner is changed during the restore phase.
4. However, backup-restore detects that the owner has changed too early in this code

   etcd-backup-restore/pkg/server/backuprestoreserver.go

   Lines 285 to 324 in eb6c516

   	if b.ownerChecker != nil {
   	// Check if the actual owner ID matches the expected one
   	// If the check returns false, take a final full snapshot if needed.
   	// If the check returns an error continue with normal operation
   	b.logger.Debugf("Checking owner before starting snapshotter...")
   	result, err := b.ownerChecker.Check(ctx)
   	if err != nil {
   	b.logger.Errorf("ownerChecker check fails: %v", err)
   	} else if !result {
   	handler.SetStatus(http.StatusServiceUnavailable)
   	// If the previous full snapshot doesn't exist or is not marked as final, take a final full snapshot
   	if ssr.PrevFullSnapshot == nil || !ssr.PrevFullSnapshot.IsFinal {
   	b.logger.Infof("Taking final full snapshot...")
   	var snapshot *brtypes.Snapshot
   	if snapshot, err = ssr.TakeFullSnapshotAndResetTimer(true); err != nil {
   	b.logger.Errorf("Could not take final full snapshot: %v", err)
   	continue
   	}
   	if b.config.HealthConfig.SnapshotLeaseRenewalEnabled {
   	leaseUpdatectx, cancel := context.WithTimeout(ctx, brtypes.LeaseUpdateTimeoutDuration)
   	defer cancel()
   	if err = heartbeat.FullSnapshotCaseLeaseUpdate(leaseUpdatectx, b.logger, snapshot, ssr.K8sClientset, b.config.HealthConfig.FullSnapshotLeaseName, b.config.HealthConfig.DeltaSnapshotLeaseName); err != nil {
   	b.logger.Warnf("Snapshot lease update failed : %v", err)
   	}
   	}
   	}
   	// Wait for the configured interval before making another attempt
   	b.logger.Infof("Waiting for %s...", b.config.OwnerCheckConfig.OwnerCheckInterval.Duration)
   	select {
   	case <-ctx.Done():
   	b.logger.Info("Shutting down...")
   	return
   	case <-time.After(b.config.OwnerCheckConfig.OwnerCheckInterval.Duration):
   	}
   	continue
   	}
   	}

   instead of here:

   etcd-backup-restore/pkg/server/backuprestoreserver.go

   Lines 442 to 456 in eb6c516

   	if b.ownerChecker != nil {
   	// Stop owner check watchdog
   	ownerCheckWatchdog.Stop()
   	// If the owner check fails or returns false, kill the etcd process
   	// to ensure that any open connections from kube-apiserver are terminated
   	result, err := b.ownerChecker.Check(ctx)
   	if err != nil {
   	b.logger.Errorf("ownerChecker check fails: %v", err)
   	} else if !result {
   	if _, err := b.etcdProcessKiller.Kill(ctx); err != nil {
   	b.logger.Errorf("Could not kill etcd process: %v", err)
   	}
   	}
   	}

   so it gets permanently stuck in lines L259-l289 and never shuts down the etcd process.

This PR adds a new boolean variable killEtcdBeforeTakingFinalSnapshot. The idea is to set it to true once the owner check has succeeded at least once, since etcd only needs to be restarted in this case - the connection from the kube-apiserver is established only in this case. If the owner check has never succeeded, then backup-restore never becomes ready and there should be no need to restart etcd

Additionally I have made it so that the final snapshot does not get taken unless the etcd process is killed. But I have some doubts about that @ishan16696 wdyt?

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:
I have tested this by locally running backup-restore and etcd and manually changing the owner DNS entry and it seems to work fine.

Release note:

When the owner check fails, `etcd-backup-restore` will restart the `etcd` process right before attempting to take a final snapshot, if the owner check was previously successful.

[ishan16696]

Overrall PR looks good, just few doubts:

1. Have you tested that seed/shoot migration scenario with this custom image of backup-restore ?

If the backup bucket is not reachable for any reason backup-restore will fail to create a snapshot

2. backup-restore won't be able to take/upload the final full snapshot. Hope that’s fine.

3. What about kill the etcd process after taking the final full snapshot ?

[plkokanov]

1. Have you tested that seed/shoot migration scenario with this custom image of backup-restore ?

Testing it currently.

2. backup-restore won't be able to take/upload the final full snapshot. Hope that’s fine.

Yep that is ok. The copy operation will just ignore it and copy w/e is currently available inside the backup directory after a 30 minute (by default) timeout. I think the only problem will be if etcd is never successfully killed - but not sure if there is anyway around it.

3. What about kill the etcd process after taking the final full snapshot ?

Don't need to kill it after taking the final snapshot if we've already killed it once before that - kube-apiserver connection should be already interrupted and the etcd-main-0 pod will be in a not ready state so that the connection is never established again.

[ishan16696]

Don't need to kill it after taking the final snapshot if we've already killed it once before that - kube-apiserver connection should be already interrupted and the etcd-main-0 pod will be in a not ready state so that the connection is never established again.

no, this is not what I meant ... I'm saying what about if we first take the full final snapshot then kill the etcd process (irrespective of final full snapshot successful or not.)
It has one advantage backup-restore don't have to wait for etcd to come back to take final full snapshot.

[plkokanov]

We always must kill it before to make sure that the connection from the kube-apiserver is interrupted and no new information is written by the kube-apiserver to the etcd while taking the final snapshot.

[ishan16696]

For testing I forgot to mention that you have to make a custom backup-restore image with these changes on the top of v0.15.3 with druid v0.8.4.
as if you use image with these changes on the top of backup-restore master then it might not work as it contains some multi-node changes and it can only work with druid v0.9.0.

[plkokanov]

For testing I forgot to mention that you have to make a custom backup-restore image with these changes on the top of v0.15.3 with druid v0.8.4.
as if you use image with these changes on the top of backup-restore master then it might not work as it contains some multi-node changes and it can only work with druid v0.9.0.

I see, thanks I was trying out with master and receiving these errors from the kube-apiserver:

0511 17:07:16.720588       1 client.go:360] parsed scheme: "endpoint"
I0511 17:07:16.720621       1 endpoint.go:68] ccResolverWrapper: sending new addresses to cc: [{https://etcd-main-client:2379  <nil> 0 <nil>}]
W0511 17:07:16.722590       1 clientconn.go:1223] grpc: addrConn.createTransport failed to connect to {https://etcd-main-client:2379  <nil> 0 <nil>}. Err :connection error: desc = "transport: authentication handshake failed: EOF". Reconnecting...
W0511 17:07:16.729505       1 clientconn.go:1223] grpc: addrConn.createTransport failed to connect to {https://etcd-main-client:2379  <nil> 0 <nil>}. Err :connection error: desc = "transport: authentication handshake failed: EOF". Reconnecting...
W0511 17:07:17.724649       1 clientconn.go:1223] grpc: addrConn.createTransport failed to connect to {https://etcd-main-client:2379  <nil> 0 <nil>}. Err :connection error: desc = "transport: authentication handshake failed: EOF". Reconnecting...
W0511 17:07:18.569912       1 clientconn.go:1223] grpc: addrConn.createTransport failed to connect to {https://etcd-main-client:2379  <nil> 0 <nil>}. Err :connection error: desc = "transport: authentication handshake failed: EOF". Reconnecting...
W0511 17:07:19.543593       1 clientconn.go:1223] grpc: addrConn.createTransport failed to connect to {https://etcd-main-client:2379  <nil> 0 <nil>}. Err :connection error: desc = "transport: authentication handshake failed: EOF". Reconnecting...
W0511 17:07:21.431953       1 clientconn.go:1223] grpc: addrConn.createTransport failed to connect to {https://etcd-main-client:2379  <nil> 0 <nil>}. Err :connection error: desc = "transport: authentication handshake failed: read tcp 10.223.131.175:60344->10.223.24.12:2379: read: connection reset by peer". Reconnecting...
W0511 17:07:22.601672       1 clientconn.go:1223] grpc: addrConn.createTransport failed to connect to {https://etcd-main-client:2379  <nil> 0 <nil>}. Err :connection error: desc = "transport: authentication handshake failed: EOF". Reconnecting...
W0511 17:07:25.492959       1 clientconn.go:1223] grpc: addrConn.createTransport failed to connect to {https://etcd-main-client:2379  <nil> 0 <nil>}. Err :connection error: desc = "transport: authentication handshake failed: EOF". Reconnecting...
W0511 17:07:26.279029       1 clientconn.go:1223] grpc: addrConn.createTransport failed to connect to {https://etcd-main-client:2379  <nil> 0 <nil>}. Err :connection error: desc = "transport: authentication handshake failed: EOF". Reconnecting...
W0511 17:07:31.455453       1 clientconn.go:1223] grpc: addrConn.createTransport failed to connect to {https://etcd-main-client:2379  <nil> 0 <nil>}. Err :connection error: desc = "transport: authentication handshake failed: EOF". Reconnecting...
W0511 17:07:33.297165       1 clientconn.go:1223] grpc: addrConn.createTransport failed to connect to {https://etcd-main-client:2379  <nil> 0 <nil>}. Err :connection error: desc = "transport: authentication handshake failed: EOF". Reconnecting...
Error: context deadline exceeded

But I wasn't sure if this wasn't because of the new secrets managed and that my shoot was pretty old. I'll try out with the versions you mentioned.

[ishan16696]

LGTM!!