From 61c7ed10dbd0ff63a3cef2c7a3554d213c633e44 Mon Sep 17 00:00:00 2001 From: Nan Yu Date: Tue, 27 Feb 2024 21:35:59 +0000 Subject: [PATCH] fix: update git-sync to v4.2.1 to fix a pulling issue (#1147) There is a bug in git-sync v4.1.0. When branches in different remotes are out of sync, `git-sync` fetches the commit SHA from the last line, which may not be the latest. This leads to an issue that Config Sync couldn't pull the latest commit from HEAD. The issue was fixed in v4.2.0 by https://github.com/kubernetes/git-sync/pull/845. This commit updates git-sync to v4.2.1 to include the fix. It also bumps the debian-base to latest version for CVE fixes. b/325341042 --- Makefile | 2 +- e2e/testcases/git_sync_test.go | 27 ++++++++----------- .../reconciler-manager-configmap.yaml | 2 +- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/Makefile b/Makefile index ce832a2219..84e53078b7 100644 --- a/Makefile +++ b/Makefile @@ -39,7 +39,7 @@ GO_DIR := $(OUTPUT_DIR)/go # Base image used for all golang containers GOLANG_IMAGE := golang:1.21.5-bookworm # Base image used for debian containers -DEBIAN_BASE_IMAGE := gcr.io/gke-release/debian-base:bookworm-v1.0.1-gke.0 +DEBIAN_BASE_IMAGE := gcr.io/gke-release/debian-base:bookworm-v1.0.1-gke.1 # Base image used for gcloud install, primarily for test images. # We use -slim for a smaller base image where we can choose which components to install. # https://cloud.google.com/sdk/docs/downloads-docker#docker_image_options diff --git a/e2e/testcases/git_sync_test.go b/e2e/testcases/git_sync_test.go index c91fb684d0..77d8d59b7f 100644 --- a/e2e/testcases/git_sync_test.go +++ b/e2e/testcases/git_sync_test.go @@ -18,7 +18,6 @@ import ( "testing" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/types" "kpt.dev/configsync/e2e/nomostest" "kpt.dev/configsync/e2e/nomostest/gitproviders" nomostesting "kpt.dev/configsync/e2e/nomostest/testing" @@ -34,15 +33,9 @@ func TestMultipleRemoteBranchesOutOfSync(t *testing.T) { if err := nt.KubeClient.Get(configsync.RootSyncName, configmanagement.ControllerNamespace, rs); err != nil { nt.T.Fatal(err) } - initialSyncedCommit := rs.Status.LastSyncedCommit nt.T.Log("Create an extra remote tracking branch") nt.Must(nt.RootRepos[configsync.RootSyncName].Push("HEAD:refs/remotes/upstream/main")) - nt.T.Cleanup(func() { - // Delete the remote tracking branch in the end so other subsequent tests - // can pull from the latest commit, instead of the HEAD of the remote. - nt.Must(nt.RootRepos[configsync.RootSyncName].Push(":refs/remotes/upstream/main")) - }) nt.T.Logf("Update the remote main branch by adding a test namespace") nt.Must(nt.RootRepos[configsync.RootSyncName].Add("acme/namespaces/hello/ns.yaml", fake.NamespaceObject("hello"))) @@ -59,16 +52,18 @@ func TestMultipleRemoteBranchesOutOfSync(t *testing.T) { nt.T.Fatal(err) } - // Apply the mitigation first to validate Config Sync couldn't pull the latest commit. - nt.T.Logf("Verify the issue exist with the default branch and revision") + nt.T.Logf("Verify git-sync can pull the latest commit with the default branch and revision") nomostest.SetGitBranch(nt, configsync.RootSyncName, gitproviders.MainBranch) - if err := nt.WatchForAllSyncs(nomostest.WithRootSha1Func( - // DefaultRootSha1Fn returns the hash with `git rev-parse HEAD`, which is - // different from `git ls-remote ...` - // So, overwrite the root hash with the initial lastSyncedCommit. - func(_ *nomostest.NT, _ types.NamespacedName) (string, error) { - return initialSyncedCommit, nil - })); err != nil { + // WatchForAllSyncs validates RootSync's lastSyncedCommit is updated to the + // local HEAD with the DefaultRootSha1Fn function. + if err := nt.WatchForAllSyncs(); err != nil { + nt.T.Fatal(err) + } + + nt.T.Logf("Remove the test namespace to make sure git-sync can fetch newer commit") + nt.Must(nt.RootRepos[configsync.RootSyncName].Remove("acme/namespaces/hello/ns.yaml")) + nt.Must(nt.RootRepos[configsync.RootSyncName].CommitAndPush("remove Namespace")) + if err := nt.WatchForAllSyncs(); err != nil { nt.T.Fatal(err) } if err := nt.ValidateNotFound("hello", "", &corev1.Namespace{}); err != nil { diff --git a/manifests/templates/reconciler-manager-configmap.yaml b/manifests/templates/reconciler-manager-configmap.yaml index c0d2011d51..0a6dddcc54 100644 --- a/manifests/templates/reconciler-manager-configmap.yaml +++ b/manifests/templates/reconciler-manager-configmap.yaml @@ -102,7 +102,7 @@ data: - NET_RAW imagePullPolicy: IfNotPresent - name: git-sync - image: gcr.io/config-management-release/git-sync:v4.1.0-gke.7__linux_amd64 + image: gcr.io/config-management-release/git-sync:v4.2.1-gke.1__linux_amd64 args: ["--root=/repo/source", "--link=rev", "--max-failures=30", "--error-file=error.json"] volumeMounts: - name: repo