Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WIP] Add watch filtering by package-id label (obsolete) #1239

Closed
wants to merge 1 commit into from
Closed

[WIP] Add watch filtering by package-id label (obsolete) #1239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 5 additions & 9 deletions e2e/testcases/custom_resources_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,16 +109,12 @@ func TestCRDDeleteBeforeRemoveCustomResourceV1(t *testing.T) {
nt.T.Fatal(err)
}

// Wait for remediator to detect the drift (managed Anvil CR was deleted)
// and record it as a conflict.
// TODO: distinguish between management conflict (spec/generation drift) and concurrent status update conflict (resource version change)
err = nomostest.ValidateMetrics(nt,
// Reverting a manual change is NOT considered a "conflict", unless the new
// owner is a different reconciler.
nt.Must(nomostest.ValidateMetrics(nt,
nomostest.ReconcilerErrorMetrics(nt, rootSyncLabels, firstCommitHash, metrics.ErrorSummary{
Conflicts: 1,
}))
if err != nil {
nt.T.Fatal(err)
}
Conflicts: 0, // from the remediator
})))

// Reset discovery client to invalidate the cached Anvil CRD
nt.RenewClient()
Expand Down
48 changes: 34 additions & 14 deletions e2e/testcases/managed_resources_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,21 @@ import (

// This file includes tests for drift correction and drift prevention.
//
// The drift correction in the mono-repo mode utilizes the following two annotations:
// * configmanagement.gke.io/managed
// * configsync.gke.io/resource-id
// The drift correction in the mono-repo mode utilizes the following metadata:
// * the configmanagement.gke.io/managed annotation
// * the configsync.gke.io/resource-id annotation
//
// The drift correction in the multi-repo mode utilizes the following three annotations:
// * configmanagement.gke.io/managed
// * configsync.gke.io/resource-id
// * configsync.gke.io/manager
// The drift correction in the multi-repo mode utilizes the following metadata:
// * the configmanagement.gke.io/managed annotation
// * the configsync.gke.io/resource-id annotation
// * the configsync.gke.io/manager annotation
// * the config.k8s.io/owning-inventory label
//
// The drift prevention is only supported in the multi-repo mode, and utilizes the following Config Sync metadata:
// The drift prevention is only supported in the multi-repo mode, and utilizes the following metadata:
// * the configmanagement.gke.io/managed annotation
// * the configsync.gke.io/resource-id annotation
// * the configsync.gke.io/declared-version label
// * the config.k8s.io/owning-inventory label

// The reason we have both TestKubectlCreatesManagedNamespaceResourceMonoRepo and
// TestKubectlCreatesManagedNamespaceResourceMultiRepo is that the mono-repo mode and
Expand Down Expand Up @@ -79,6 +81,8 @@ metadata:
configmanagement.gke.io/managed: enabled
configsync.gke.io/resource-id: _namespace_test-ns1
configsync.gke.io/manager: :root
labels:
configsync.gke.io/parent-package-id: root-sync.config-management-system.RootSync
`)

if err := os.WriteFile(filepath.Join(nt.TmpDir, "test-ns1.yaml"), ns, 0644); err != nil {
Expand Down Expand Up @@ -106,7 +110,9 @@ metadata:
annotations:
configmanagement.gke.io/managed: enabled
configsync.gke.io/resource-id: _namespace_test-ns2
configsync.gke.io/manager: :abcdef
configsync.gke.io/manager: config-management-system_abcdef
labels:
configsync.gke.io/parent-package-id: abcdef.config-management-system.RootSync
`)

if err := os.WriteFile(filepath.Join(nt.TmpDir, "test-ns2.yaml"), ns, 0644); err != nil {
Expand All @@ -121,8 +127,10 @@ metadata:
// Wait 5 seconds so that the remediator can process the event.
time.Sleep(5 * time.Second)

// The `configsync.gke.io/manager` annotation of `test-ns2` suggests that its manager is ':abcdef'.
// The root reconciler does not manage `test-ns2`, therefore should not remove `test-ns2`.
// The `configsync.gke.io/manager` annotation of `test-ns2` suggests that
// its manager is 'config-management-system_abcdef' (RootSync named 'abcdef').
// The root reconciler (RootSync named 'root-sync') does not manage
// `test-ns2`, therefore should not remove `test-ns2`.
err = nt.Validate("test-ns2", "", &corev1.Namespace{},
testpredicates.HasExactlyAnnotationKeys(
metadata.ResourceManagementKey,
Expand Down Expand Up @@ -233,6 +241,8 @@ metadata:
configmanagement.gke.io/managed: enabled
configsync.gke.io/resource-id: _configmap_bookstore_test-cm1
configsync.gke.io/manager: :root
labels:
configsync.gke.io/parent-package-id: root-sync.config-management-system.RootSync
data:
weekday: "monday"
`)
Expand Down Expand Up @@ -263,6 +273,8 @@ metadata:
configmanagement.gke.io/managed: enabled
configsync.gke.io/resource-id: _configmap_bookstore_wrong-cm2
configsync.gke.io/manager: :root
labels:
configsync.gke.io/parent-package-id: root-sync.config-management-system.RootSync
data:
weekday: "monday"
`)
Expand Down Expand Up @@ -301,7 +313,9 @@ metadata:
annotations:
configmanagement.gke.io/managed: enabled
configsync.gke.io/resource-id: _configmap_bookstore_test-cm3
configsync.gke.io/manager: :abcdef
configsync.gke.io/manager: config-management-system_abcdef
labels:
configsync.gke.io/parent-package-id: abcdef.config-management-system.RootSync
data:
weekday: "monday"
`)
Expand All @@ -318,8 +332,10 @@ data:
// Wait 5 seconds so that the reconciler can process the event.
time.Sleep(5 * time.Second)

// The `configsync.gke.io/manager` annotation of `test-ns3` suggests that its manager is ':abcdef'.
// The root reconciler does not manage `test-ns3`, therefore should not remove `test-ns3`.
// The `configsync.gke.io/manager` annotation of `test-ns3` suggests that
// its manager is 'config-management-system_abcdef' (RootSync named 'abcdef').
// The root reconciler (RootSync named 'root-sync') does not manage `test-ns3`,
// therefore should not remove `test-ns3`.
err = nt.Validate("test-cm3", "bookstore", &corev1.ConfigMap{},
testpredicates.HasExactlyAnnotationKeys(
metadata.ResourceManagementKey,
Expand All @@ -340,6 +356,8 @@ metadata:
annotations:
configmanagement.gke.io/managed: enabled
configsync.gke.io/resource-id: _configmap_bookstore_test-cm4
labels:
configsync.gke.io/parent-package-id: root-sync.config-management-system.RootSync
data:
weekday: "monday"
`)
Expand Down Expand Up @@ -378,6 +396,8 @@ metadata:
configmanagement.gke.io/managed: enabled
configsync.gke.io/resource-id: _configmap_bookstore_test-secret
configsync.gke.io/manager: :root
labels:
configsync.gke.io/parent-package-id: root-sync.config-management-system.RootSync
`)

if err := os.WriteFile(filepath.Join(nt.TmpDir, "test-secret.yaml"), cm, 0644); err != nil {
Expand Down
99 changes: 39 additions & 60 deletions e2e/testcases/multi_sync_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,122 +396,101 @@ func TestConflictingDefinitions_NamespaceToRoot(t *testing.T) {
)

podRoleFilePath := fmt.Sprintf("acme/namespaces/%s/pod-role.yaml", testNs)
nt.T.Logf("Add a Role to Namespace repo: %s", configsync.RootSyncName)
nt.T.Logf("Add a Role to Namespace repo: %v", repoSyncNN)
nsRoleObj := namespacePodRole()
nt.Must(nt.NonRootRepos[repoSyncNN].Add(podRoleFilePath, nsRoleObj))
nt.Must(nt.NonRootRepos[repoSyncNN].CommitAndPush("declare Role"))
if err := nt.WatchForAllSyncs(); err != nil {
nt.T.Fatal(err)
}
nt.Must(nt.WatchForAllSyncs())

err := nt.Validate("pods", testNs, &rbacv1.Role{},
// Validate Role is managed by the RepoSync reconciler
nt.Must(nt.Validate(nsRoleObj.Name, nsRoleObj.Namespace, &rbacv1.Role{},
roleHasRules(nsRoleObj.Rules),
testpredicates.IsManagedBy(nt.Scheme, declared.Scope(repoSyncNN.Namespace), repoSyncNN.Name))
if err != nil {
nt.T.Fatal(err)
}
testpredicates.IsManagedBy(nt.Scheme, declared.Scope(repoSyncNN.Namespace), repoSyncNN.Name)))

// Test Role managed by the RepoSync, NOT the RootSync
nt.MetricsExpectations.AddObjectApply(configsync.RepoSyncKind, repoSyncNN, nsRoleObj)

// Validate no errors from root reconciler.
err = nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{
nt.Must(nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{
Sync: rootSyncNN,
})
if err != nil {
nt.T.Fatal(err)
}
}))

// Validate no errors from namespace reconciler.
err = nomostest.ValidateStandardMetricsForRepoSync(nt, metrics.Summary{
nt.Must(nomostest.ValidateStandardMetricsForRepoSync(nt, metrics.Summary{
Sync: repoSyncNN,
})
if err != nil {
nt.T.Fatal(err)
}
}))

nt.T.Logf("Declare a conflicting Role in the Root repo: %s", configsync.RootSyncName)
rootRoleObj := rootPodRole()
nt.Must(nt.RootRepos[configsync.RootSyncName].Add(podRoleFilePath, rootRoleObj))
nt.Must(nt.RootRepos[configsync.RootSyncName].CommitAndPush("add conflicting pod role to Root"))

nt.T.Logf("The RootSync should update the Role")
err = nt.WatchForAllSyncs(nomostest.RootSyncOnly())
if err != nil {
nt.T.Fatal(err)
}
nt.Must(nt.WatchForAllSyncs(nomostest.RootSyncOnly()))

// The RepoSync remediator won't try to revert a change to a managed object
// if it was adopted by a RepoSync reconciler.
// So without a new commit or pod replacement, the reconciler won't
// automatically re-sync until the ~1hr resync period expires.
// nt.T.Logf("Push a new commit with an empty file to trigger re-sync by the RepoSync reconciler")
// nt.Must(nt.NonRootRepos[repoSyncNN].AddFile("acme/empty.txt", []byte{}))
// nt.Must(nt.NonRootRepos[repoSyncNN].CommitAndPush("declare Role"))

nt.T.Logf("The RepoSync %s reports a problem since it can't sync the declaration.", testNs)
nt.WaitForRepoSyncSyncError(repoSyncNN.Namespace, repoSyncNN.Name, status.ManagementConflictErrorCode, "declared in another repository", nil)

nt.MetricsExpectations.AddObjectApply(configsync.RootSyncKind, rootSyncNN, rootRoleObj)

// Validate no errors from root reconciler.
err = nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{
nt.Must(nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{
Sync: rootSyncNN,
})
if err != nil {
nt.T.Fatal(err)
}
}))

// Validate reconciler error metric is emitted from namespace reconciler.
rootSyncLabels, err := nomostest.MetricLabelsForRepoSync(nt, repoSyncNN)
repoSyncLabels, err := nomostest.MetricLabelsForRepoSync(nt, repoSyncNN)
if err != nil {
nt.T.Fatal(err)
}
commitHash := nt.NonRootRepos[repoSyncNN].MustHash(nt.T)

err = nomostest.ValidateMetrics(nt,
nomostest.ReconcilerSyncError(nt, rootSyncLabels, commitHash),
nomostest.ReconcilerErrorMetrics(nt, rootSyncLabels, commitHash, metrics.ErrorSummary{
Sync: 1,
}))
if err != nil {
nt.T.Fatal(err)
}
// Reverting a manual change IS considered a "conflict" when the new
// owner is a different reconciler.
nt.Must(nomostest.ValidateMetrics(nt,
nomostest.ReconcilerSyncError(nt, repoSyncLabels, commitHash),
nomostest.ReconcilerErrorMetrics(nt, repoSyncLabels, commitHash, metrics.ErrorSummary{
Conflicts: 1, // from the remediator
Sync: 1, // remediator conflict error replaces applier conflict error
})))

nt.T.Logf("Ensure the Role matches the one in the Root repo %s", configsync.RootSyncName)
err = nt.Validate("pods", testNs, &rbacv1.Role{},
nt.Must(nt.Validate(nsRoleObj.Name, nsRoleObj.Namespace, &rbacv1.Role{},
roleHasRules(rootPodRole().Rules),
testpredicates.IsManagedBy(nt.Scheme, declared.RootScope, configsync.RootSyncName))
if err != nil {
nt.T.Fatal(err)
}
testpredicates.IsManagedBy(nt.Scheme, declared.RootScope, configsync.RootSyncName)))

nt.T.Logf("Remove the Role from the Namespace repo %s", repoSyncNN)
nt.Must(nt.NonRootRepos[repoSyncNN].Remove(podRoleFilePath))
nt.Must(nt.NonRootRepos[repoSyncNN].CommitAndPush("remove conflicting pod role from Namespace repo"))
if err := nt.WatchForAllSyncs(); err != nil {
nt.T.Fatal(err)
}
nt.Must(nt.WatchForAllSyncs())

nt.T.Logf("Ensure the Role still matches the one in the Root repo %s", configsync.RootSyncName)
err = nt.Validate("pods", testNs, &rbacv1.Role{},
nt.Must(nt.Validate(nsRoleObj.Name, nsRoleObj.Namespace, &rbacv1.Role{},
roleHasRules(rootPodRole().Rules),
testpredicates.IsManagedBy(nt.Scheme, declared.RootScope, configsync.RootSyncName))
if err != nil {
nt.T.Fatal(err)
}
testpredicates.IsManagedBy(nt.Scheme, declared.RootScope, configsync.RootSyncName)))

// Test Role managed by the RootSync, NOT the RepoSync
nt.MetricsExpectations.AddObjectApply(configsync.RootSyncKind, rootSyncNN, rootRoleObj)
// RepoSync won't delete the object, because it doesn't own it, due to the AdoptIfNoInventory strategy.
nt.MetricsExpectations.RemoveObject(configsync.RepoSyncKind, repoSyncNN, nsRoleObj)

// Validate no errors from root reconciler.
err = nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{
nt.Must(nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{
Sync: rootSyncNN,
})
if err != nil {
nt.T.Fatal(err)
}
}))

// Validate no errors from namespace reconciler.
err = nomostest.ValidateStandardMetricsForRepoSync(nt, metrics.Summary{
nt.Must(nomostest.ValidateStandardMetricsForRepoSync(nt, metrics.Summary{
Sync: repoSyncNN,
})
if err != nil {
nt.T.Fatal(err)
}
}))
}

func TestConflictingDefinitions_RootToRoot(t *testing.T) {
Expand Down
16 changes: 12 additions & 4 deletions e2e/testcases/preserve_fields_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,11 @@ func TestAddUpdateDeleteLabels(t *testing.T) {
nt.T.Fatal(err)
}

var defaultLabels = []string{metadata.ManagedByKey, metadata.DeclaredVersionLabel}
var defaultLabels = []string{
metadata.ManagedByKey,
metadata.ParentPackageIDLabel,
metadata.DeclaredVersionLabel,
}

// Checking that the configmap with no labels appears on cluster, and
// that no user labels are specified
Expand All @@ -350,9 +354,13 @@ func TestAddUpdateDeleteLabels(t *testing.T) {
nt.T.Fatal(err)
}

// Checking that label is updated after syncing an update.
var updatedLabels []string
updatedLabels = append(updatedLabels, defaultLabels...)
updatedLabels = append(updatedLabels, "baz")

// Checking that label was added after syncing.
err = nt.Validate(cmName, ns, &corev1.ConfigMap{},
testpredicates.HasExactlyLabelKeys(append(defaultLabels, "baz")...))
testpredicates.HasExactlyLabelKeys(updatedLabels...))
if err != nil {
nt.T.Fatal(err)
}
Expand All @@ -366,7 +374,7 @@ func TestAddUpdateDeleteLabels(t *testing.T) {

// Check that the label is deleted after syncing.
err = nt.Validate(cmName, ns, &corev1.ConfigMap{},
testpredicates.HasExactlyLabelKeys(metadata.ManagedByKey, metadata.DeclaredVersionLabel))
testpredicates.HasExactlyLabelKeys(defaultLabels...))
if err != nil {
nt.T.Fatal(err)
}
Expand Down
Loading