Skip to content

Commit

Permalink
Remove legacy reconciler template test artifact (#767)
Browse files Browse the repository at this point in the history
The old reconciler template has confused multiple people now.
So instead of keeping it to test that the reconciler template can
be changed, pull the current template in the test, modify it, and
persist the change. This way we can test a single-line change of
a managed field is reverted without needing the old config to keep
working in perpetuity. Now there's only one template in the repo.
  • Loading branch information
karlkfi authored Jul 27, 2023
1 parent 61508ce commit 6c45486
Show file tree
Hide file tree
Showing 7 changed files with 212 additions and 470 deletions.
8 changes: 4 additions & 4 deletions e2e/nomostest/config_sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,16 +95,16 @@ var (
// IsReconcilerTemplateConfigMap returns true if passed obj is the
// reconciler-manager ConfigMap reconciler-manager-cm in config-management namespace.
func IsReconcilerTemplateConfigMap(obj client.Object) bool {
return obj.GetName() == "reconciler-manager-cm" &&
obj.GetNamespace() == "config-management-system" &&
return obj.GetName() == controllers.ReconcilerTemplateConfigMapName &&
obj.GetNamespace() == configmanagement.ControllerNamespace &&
obj.GetObjectKind().GroupVersionKind() == kinds.ConfigMap()
}

// IsReconcilerManagerConfigMap returns true if passed obj is the
// reconciler-manager ConfigMap reconciler-manager in config-management namespace.
func IsReconcilerManagerConfigMap(obj client.Object) bool {
return obj.GetName() == "reconciler-manager" &&
obj.GetNamespace() == "config-management-system" &&
return obj.GetName() == reconcilermanager.ManagerName &&
obj.GetNamespace() == configmanagement.ControllerNamespace &&
obj.GetObjectKind().GroupVersionKind() == kinds.ConfigMap()
}

Expand Down
181 changes: 116 additions & 65 deletions e2e/nomostest/testpredicates/predicates.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,25 +228,24 @@ func HasExactlyImage(containerName, expectImageName, expectImageTag, expectImage
if !ok {
return WrongTypeErr(dep, &appsv1.Deployment{})
}
for _, container := range dep.Spec.Template.Spec.Containers {
if containerName == container.Name {
expectImage := ""
if expectImageName != "" {
expectImage = "/" + expectImageName
}
if expectImageTag != "" {
expectImage += ":" + expectImageTag
}
if expectImageDigest != "" {
expectImage += "@" + expectImageDigest
}
if !strings.Contains(container.Image, expectImage) {
return errors.Errorf("Expected %q container image contains %q, however the actual image is %q", container.Name, expectImage, container.Image)
}
return nil
}
container := ContainerByName(dep, containerName)
if container == nil {
return fmt.Errorf("expected container not found: %s", containerName)
}
expectImage := ""
if expectImageName != "" {
expectImage = "/" + expectImageName
}
if expectImageTag != "" {
expectImage += ":" + expectImageTag
}
return errors.Errorf("Container %q not found", containerName)
if expectImageDigest != "" {
expectImage += "@" + expectImageDigest
}
if !strings.Contains(container.Image, expectImage) {
return errors.Errorf("Expected %q container image contains %q, however the actual image is %q", container.Name, expectImage, container.Image)
}
return nil
}
}

Expand All @@ -260,25 +259,23 @@ func HasCorrectResourceRequestsLimits(containerName string, cpuRequest, cpuLimit
if !ok {
return WrongTypeErr(dep, &appsv1.Deployment{})
}
for _, container := range dep.Spec.Template.Spec.Containers {
if containerName == container.Name {
if !equality.Semantic.DeepEqual(container.Resources.Requests[corev1.ResourceCPU], cpuRequest) {
return errors.Errorf("The CPU request of the %q container should be %v, got %v", container.Name, cpuRequest, container.Resources.Requests[corev1.ResourceCPU])
}
if !equality.Semantic.DeepEqual(container.Resources.Limits[corev1.ResourceCPU], cpuLimit) {
return errors.Errorf("The CPU limit of the %q container should be %v, got %v", container.Name, cpuLimit, container.Resources.Limits[corev1.ResourceCPU])
}
if !equality.Semantic.DeepEqual(container.Resources.Requests[corev1.ResourceMemory], memoryRequest) {
return errors.Errorf("The memory request of the %q container should be %v, got %v", container.Name, memoryRequest, container.Resources.Requests[corev1.ResourceMemory])
}
if !equality.Semantic.DeepEqual(container.Resources.Limits[corev1.ResourceMemory], memoryLimit) {
return errors.Errorf("The memory limit of the %q container should be %v, got %v", container.Name, memoryLimit, container.Resources.Limits[corev1.ResourceMemory])
}

return nil
}
container := ContainerByName(dep, containerName)
if container == nil {
return fmt.Errorf("expected container not found: %s", containerName)
}
if !equality.Semantic.DeepEqual(container.Resources.Requests[corev1.ResourceCPU], cpuRequest) {
return errors.Errorf("The CPU request of the %q container should be %v, got %v", container.Name, cpuRequest, container.Resources.Requests[corev1.ResourceCPU])
}
if !equality.Semantic.DeepEqual(container.Resources.Limits[corev1.ResourceCPU], cpuLimit) {
return errors.Errorf("The CPU limit of the %q container should be %v, got %v", container.Name, cpuLimit, container.Resources.Limits[corev1.ResourceCPU])
}
return errors.Errorf("Container %q not found", containerName)
if !equality.Semantic.DeepEqual(container.Resources.Requests[corev1.ResourceMemory], memoryRequest) {
return errors.Errorf("The memory request of the %q container should be %v, got %v", container.Name, memoryRequest, container.Resources.Requests[corev1.ResourceMemory])
}
if !equality.Semantic.DeepEqual(container.Resources.Limits[corev1.ResourceMemory], memoryLimit) {
return errors.Errorf("The memory limit of the %q container should be %v, got %v", container.Name, memoryLimit, container.Resources.Limits[corev1.ResourceMemory])
}
return nil
}
}

Expand Down Expand Up @@ -386,19 +383,18 @@ func DeploymentHasEnvVar(containerName, key, value string) Predicate {
if !ok {
return WrongTypeErr(o, d)
}
for _, c := range d.Spec.Template.Spec.Containers {
if c.Name == containerName {
for _, e := range c.Env {
if e.Name == key {
if e.Value == value {
return nil
}
return errors.Errorf("Container %q has the wrong value for environment variable %q. Expected : %q, actual %q", containerName, key, value, e.Value)
}
container := ContainerByName(d, containerName)
if container == nil {
return fmt.Errorf("expected container not found: %s", containerName)
}
for _, e := range container.Env {
if e.Name == key {
if e.Value == value {
return nil
}
return errors.Errorf("Container %q has the wrong value for environment variable %q. Expected : %q, actual %q", containerName, key, value, e.Value)
}
}

return errors.Errorf("Container %q does not contain environment variable %q", containerName, key)
}
}
Expand All @@ -414,16 +410,15 @@ func DeploymentMissingEnvVar(containerName, key string) Predicate {
if !ok {
return WrongTypeErr(o, d)
}
for _, c := range d.Spec.Template.Spec.Containers {
if c.Name == containerName {
for _, e := range c.Env {
if e.Name == key {
return errors.Errorf("Container %q should not have environment variable %q", containerName, key)
}
}
container := ContainerByName(d, containerName)
if container == nil {
return fmt.Errorf("expected container not found: %s", containerName)
}
for _, e := range container.Env {
if e.Name == key {
return errors.Errorf("Container %q should not have environment variable %q", containerName, key)
}
}

return nil
}
}
Expand All @@ -439,13 +434,12 @@ func DeploymentHasContainer(containerName string) Predicate {
if !ok {
return WrongTypeErr(o, d)
}
nn := core.ObjectNamespacedName(o)
for _, c := range d.Spec.Template.Spec.Containers {
if c.Name == containerName {
return nil
}
container := ContainerByName(d, containerName)
if container == nil {
return errors.Errorf("Deployment %s should have container %s",
core.ObjectNamespacedName(o), containerName)
}
return errors.Errorf("Deployment %s should have container %s", nn, containerName)
return nil
}
}

Expand All @@ -460,11 +454,10 @@ func DeploymentMissingContainer(containerName string) Predicate {
if !ok {
return WrongTypeErr(o, d)
}
nn := core.ObjectNamespacedName(o)
for _, c := range d.Spec.Template.Spec.Containers {
if c.Name == containerName {
return errors.Errorf("Deployment %s should not have container %s", nn, containerName)
}
container := ContainerByName(d, containerName)
if container != nil {
return errors.Errorf("Deployment %s should not have container %s",
core.ObjectNamespacedName(o), containerName)
}
return nil
}
Expand Down Expand Up @@ -894,3 +887,61 @@ func WatchSyncPredicate() (Predicate, <-chan struct{}) {
return nil
}, syncCh
}

// DeploymentContainerImageEquals returns a predicate that errors if the
// deployment does not have a container with the specified name and image.
func DeploymentContainerImageEquals(containerName, image string) Predicate {
return func(o client.Object) error {
if o == nil {
return ErrObjectNotFound
}
d, ok := o.(*appsv1.Deployment)
if !ok {
return WrongTypeErr(d, &appsv1.Deployment{})
}
container := ContainerByName(d, containerName)
if container == nil {
return fmt.Errorf("expected container not found: %s", containerName)
}
if container.Image != image {
return fmt.Errorf("expected %q container image to equal: %s, got: %s",
containerName, image, container.Image)
}
return nil
}
}

// DeploymentContainerPullPolicyEquals returns a predicate that errors if the
// deployment does not have a container with the specified name and
// imagePullPolicy.
func DeploymentContainerPullPolicyEquals(containerName string, policy corev1.PullPolicy) Predicate {
return func(o client.Object) error {
if o == nil {
return ErrObjectNotFound
}
d, ok := o.(*appsv1.Deployment)
if !ok {
return WrongTypeErr(d, &appsv1.Deployment{})
}
container := ContainerByName(d, containerName)
if container == nil {
return fmt.Errorf("expected container not found: %s", containerName)
}
if container.ImagePullPolicy != policy {
return fmt.Errorf("expected %q container imagePullPolicy to equal: %s, got: %s",
containerName, policy, container.ImagePullPolicy)
}
return nil
}
}

// ContainerByName returns a copy of the container with the specified name,
// found in the specified Deployment.
func ContainerByName(obj *appsv1.Deployment, containerName string) *corev1.Container {
for _, container := range obj.Spec.Template.Spec.Containers {
if container.Name == containerName {
return container.DeepCopy()
}
}
return nil
}
11 changes: 7 additions & 4 deletions e2e/testcases/helm_sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ func TestPublicHelm(t *testing.T) {
expectedMemoryRequest = "250Mi"
expectedMemoryLimit = "300Mi"
}
if err := nt.Validate("my-wordpress", "wordpress", &appsv1.Deployment{}, containerImagePullPolicy("Always"),
if err := nt.Validate("my-wordpress", "wordpress", &appsv1.Deployment{},
testpredicates.DeploymentContainerPullPolicyEquals("wordpress", "Always"),
testpredicates.HasCorrectResourceRequestsLimits("wordpress",
resource.MustParse(expectedCPURequest),
resource.MustParse(expectedCPULimit),
Expand Down Expand Up @@ -272,7 +273,8 @@ service:
expectedMemoryRequest = "250Mi"
expectedMemoryLimit = "300Mi"
}
if err := nt.Validate("my-wordpress", "wordpress", &appsv1.Deployment{}, containerImagePullPolicy("Always"),
if err := nt.Validate("my-wordpress", "wordpress", &appsv1.Deployment{},
testpredicates.DeploymentContainerPullPolicyEquals("wordpress", "Always"),
testpredicates.HasCorrectResourceRequestsLimits("wordpress",
resource.MustParse(expectedCPURequest),
resource.MustParse(expectedCPULimit),
Expand Down Expand Up @@ -364,7 +366,8 @@ image:
}

// duplicated keys from later files should override the keys from previous files.
if err := nt.Validate("my-wordpress", "wordpress", &appsv1.Deployment{}, containerImagePullPolicy("Always"),
if err := nt.Validate("my-wordpress", "wordpress", &appsv1.Deployment{},
testpredicates.DeploymentContainerPullPolicyEquals("wordpress", "Always"),
testpredicates.HasExactlyImage("wordpress", "bitnami/wordpress", "", "sha256:362cb642db481ebf6f14eb0244fbfb17d531a84ecfe099cd3bba6810db56694e"),
testpredicates.DeploymentHasEnvVar("wordpress", "WORDPRESS_USERNAME", "test-user-2"),
testpredicates.DeploymentHasEnvVar("wordpress", "WORDPRESS_EMAIL", "[email protected]"),
Expand Down Expand Up @@ -733,7 +736,7 @@ func TestHelmGCENode(t *testing.T) {
nt.T.Fatal(err)
}
if err := nt.Validate(fmt.Sprintf("my-coredns-%s", remoteHelmChart.ChartName), "coredns", &appsv1.Deployment{},
containerImagePullPolicy("IfNotPresent")); err != nil {
testpredicates.DeploymentContainerPullPolicyEquals("coredns", "IfNotPresent")); err != nil {
nt.T.Error(err)
}
}
Expand Down
26 changes: 5 additions & 21 deletions e2e/testcases/hydration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,8 @@ func TestHydrateHelmComponents(t *testing.T) {
nt.T.Fatal(err)
}
if err := nt.Validate("my-coredns-coredns", "coredns", &appsv1.Deployment{},
containerImagePullPolicy("Always"), firstContainerImageIs("coredns/coredns:1.8.4"),
testpredicates.DeploymentContainerPullPolicyEquals("coredns", "Always"),
testpredicates.DeploymentContainerImageEquals("coredns", "coredns/coredns:1.8.4"),
testpredicates.HasAnnotation(metadata.KustomizeOrigin, expectedBuiltinOrigin)); err != nil {
nt.T.Fatal(err)
}
Expand Down Expand Up @@ -474,24 +475,6 @@ func validateNamespaces(nt *nomostest.NT, expectedNamespaces []string, expectedO
}
}

func containerImagePullPolicy(policy string) testpredicates.Predicate {
return func(o client.Object) error {
if o == nil {
return testpredicates.ErrObjectNotFound
}
rq, ok := o.(*appsv1.Deployment)
if !ok {
return testpredicates.WrongTypeErr(rq, &appsv1.Deployment{})
}

actual := rq.Spec.Template.Spec.Containers[0].ImagePullPolicy
if policy != string(actual) {
return fmt.Errorf("container policy %q is not equal to the expected %q", actual, policy)
}
return nil
}
}

// getRootSyncCommitStatusErrorSummary converts the given rootSync into a RepoState, then into a string
func getRootSyncCommitStatusErrorSummary(rootSync *v1beta1.RootSync, rg *unstructured.Unstructured, syncingConditionSupported bool) (string, string, string) {
rs := nomosstatus.RootRepoStatus(rootSync, rg, syncingConditionSupported)
Expand Down Expand Up @@ -553,13 +536,14 @@ func validateTenant(nt *nomostest.NT, reconcilerScope, tenant, baseRelPath strin
func validateHelmComponents(nt *nomostest.NT, reconcilerScope string) {
nt.T.Log("Validate resources are synced")
if err := nt.Validate("my-coredns-coredns", "coredns", &appsv1.Deployment{},
containerImagePullPolicy("IfNotPresent"),
testpredicates.DeploymentContainerPullPolicyEquals("coredns", "IfNotPresent"),
testpredicates.HasAnnotation(metadata.KustomizeOrigin, expectedBuiltinOrigin),
testpredicates.HasAnnotation(metadata.ResourceManagerKey, reconcilerScope)); err != nil {
nt.T.Error(err)
}
if err := nt.Validate("my-wordpress", "wordpress",
&appsv1.Deployment{}, containerImagePullPolicy("IfNotPresent"),
&appsv1.Deployment{},
testpredicates.DeploymentContainerPullPolicyEquals("wordpress", "IfNotPresent"),
testpredicates.HasAnnotation(metadata.KustomizeOrigin, expectedBuiltinOrigin),
testpredicates.HasAnnotation(metadata.ResourceManagerKey, reconcilerScope)); err != nil {
nt.T.Error(err)
Expand Down
Loading

0 comments on commit 6c45486

Please sign in to comment.