Follow-up nits from etcd init script pull request

This is a follow on from the review of
cilium#29109, and is a collection of
minor changes:
- Remove unused variables in install/kubernetes Makefile values file
- Remove etcd image from check-docker-images.sh script
- Remove now-unused external dependencies block from
  check-docker-images.sh script
- Clarify doc comment for ClusterMeshEtcdInit function, to correctly
  state the purpose of the hasConfig key
- defer etcdClient.Close() after creating etcdClient
- Use errors.Join to handle errors in defered cleanup code, where the
  main function may have also returned an error
- Correct typo in comment: "it's" to "its"

Signed-off-by: James Laverack <james@isovalent.com>

clustermesh-apiserver/etcdinit/root.go

@@ -5,6 +5,7 @@ package etcdinit
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	"os/exec"
@@ -154,14 +155,16 @@ func InitEtcdLocal() (returnErr error) {
 	defer func() {
 		log := log.WithField("etcdPID", etcdPid)
 		log.Debug("Cleaning up etcd process")
-		// Send the process a SIGTERM. SIGTERM is the "gentle" shutdown signal, and etcd should close down it's resources
+		// Send the process a SIGTERM. SIGTERM is the "gentle" shutdown signal, and etcd should close down its resources
 		// cleanly and then exit.
 		log.Info("Sending SIGTERM signal to etcd process")
 		err := etcdCmd.Process.Signal(syscall.SIGTERM)
 		if err != nil {
 			log.WithError(err).
 				Error("Failed to send SIGTERM signal to etcd process")
-			returnErr = err
+			// Return both this error, and the main function's return error (if there is one).
+			returnErr = errors.Join(returnErr, err)
+			return
 		}
 
 		// Wait for the etcd process to finish, and cleanup resources.
@@ -179,10 +182,9 @@ func InitEtcdLocal() (returnErr error) {
 						// which would report a false error. That's very unlikely, so we don't worry about it here.
 						log.WithField("timeout", timeout).
 							Error("etcd exited, but our context has expired. etcd may have been terminated due to timeout. Consider increasing the value of the timeout using the --timeout flag or CILIUM_TIMEOUT environment variable.")
-						// If we're not already returning an error, return an error here
-						if returnErr == nil {
-							returnErr = ctx.Err()
-						}
+						// Return both this error, and the main function's return error (if there is one). This is just
+						// to make sure that the calling code correctly detects that an error occurs.
+						returnErr = errors.Join(returnErr, ctx.Err())
 						return
 					}
 					// This is the "good state", the context hasn't expired, the etcd process has exited, and we're
@@ -193,19 +195,15 @@ func InitEtcdLocal() (returnErr error) {
 				log.WithError(err).
 					WithField("etcdExitCode", exitError.ExitCode()).
 					Error("etcd process exited improperly")
-				// If we're not already returning an error, return an error here
-				if returnErr == nil {
-					returnErr = err
-				}
+				// Return both this error, and the main function's return error (if there is one).
+				returnErr = errors.Join(returnErr, err)
 				return
 			} else {
 				// Some other kind of error
 				log.WithError(err).
 					Error("Failed to wait on etcd process finishing")
-				// If we're not already returning an error, return an error here
-				if returnErr == nil {
-					returnErr = err
-				}
+				// Return both this error, and the main function's return error (if there is one).
+				returnErr = errors.Join(returnErr, err)
 				return
 			}
 		}
@@ -226,6 +224,7 @@ func InitEtcdLocal() (returnErr error) {
 			Error("Failed to construct etcd client from configuration")
 		return err
 	}
+	defer etcdClient.Close()
 
 	// Run the init commands
 	log.WithField(logfields.ClusterName, ciliumClusterName).

contrib/release/check-docker-images.sh

@@ -6,10 +6,6 @@ cilium_tag="${1}"
 org="cilium"
 suffix=${CILIUM_OPERATOR_SUFFIX:-}
 
-external_dependencies=(
-  "quay.io/coreos/etcd:${ETCD_VERSION}" \
-)
-
 internal_dependencies_quay_only=(
   "cilium-etcd-operator:${CILIUM_ETCD_OPERATOR_VERSION}" \
   "startup-script:${CILIUM_NODEINIT_VERSION}"
@@ -37,13 +33,6 @@ image_tag_exists(){
   docker buildx imagetools inspect "${image}" &> /dev/null
 }
 
-for image in "${external_dependencies[@]}" ; do
-  if ! image_tag_exists "${image}" ; then
-    echo "${image} does not exist!"
-    not_found=true
-  fi
-done
-
 for image in "${internal_dependencies[@]}" ; do
   image_tag=${image#*:}
   image_name=${org}/${image%":$image_tag"}

install/kubernetes/Makefile.values

@@ -44,10 +44,6 @@ export CILIUM_ENVOY_REPO:=quay.io/cilium/cilium-envoy
 export CILIUM_ENVOY_VERSION:=v1.27.2-f19708f3d0188fe39b7e024b4525b75a9eeee61f
 export CILIUM_ENVOY_DIGEST:=sha256:80de27c1d16ab92923cc0cd1fff90f2e7047a9abf3906fda712268d9cbc5b950
 
-export ETCD_REPO:=quay.io/coreos/etcd
-export ETCD_VERSION:=v3.5.4
-export ETCD_DIGEST:=sha256:795d8660c48c439a7c3764c2330ed9222ab5db5bb524d8d0607cac76f7ba82a3
-
 export HUBBLE_UI_BACKEND_REPO:=quay.io/cilium/hubble-ui-backend
 export HUBBLE_UI_BACKEND_VERSION:=v0.12.1
 export HUBBLE_UI_BACKEND_DIGEST:=sha256:1f86f3400827a0451e6332262467f894eeb7caf0eb8779bd951e2caa9d027cbe

pkg/kvstore/etcdinit/init.go

@@ -17,8 +17,8 @@ import (
 )
 
 // ClusterMeshEtcdInit initializes etcd for use by Cilium Clustermesh via the provided client. It creates a number of
-// user accounts and roles with permissions, sets a well-known key to indicate config has been done, and enables
-// authentication for the cluster.
+// user accounts and roles with permissions, sets a well-known key to indicate that clients should expect a cilium
+// config to be present, and enables authentication for the cluster.
 //
 // This function uses log to perform informational and debug logging about operations. This function does not log errors
 // and instead returns an error for handling, as it is assumed that the calling function will log errors. Most errors