Skip to content
This repository was archived by the owner on Aug 11, 2021. It is now read-only.

Add service acceptance tests #198

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add service acceptance tests #198

wants to merge 4 commits into from

Conversation

redeux
Copy link
Contributor

@redeux redeux commented Apr 30, 2021

Description

Tests for the Kubernetes Service resource.

Includes test for types:

  • LoadBalancer
  • ClusterIP
  • NodePort
  • ExternalName
  • None (ClusterIP)

References

https://kubernetes.io/docs/reference/kubernetes-api/service-resources/service-v1/

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

@ghost ghost added the size/L label Apr 30, 2021
@ghost ghost added size/XL and removed size/L labels Apr 30, 2021
@redeux redeux marked this pull request as ready for review April 30, 2021 22:43
Copy link
Member

@alexsomesan alexsomesan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet! Really love these! Nicely done, @redeux !

One thing I wanted to mention before we merge: in the case of ClusterIP there are some actual server-generated attribute values (the IP addresses) that would be useful to capture in assertions. See my comment below for the whole story.

"kubernetes_manifest.test.object.spec.ports.0.targetPort": json.Number("8080"),
"kubernetes_manifest.test.object.spec.ports.0.protocol": "TCP",
"kubernetes_manifest.test.object.spec.selector.app": "test",
"kubernetes_manifest.test.object.spec.type": "ClusterIP",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a service of type "ClusterIP" is created, the resulting resource gets two attributes added by the controller manager which hold the actual cluster IP address(es) allocated to the service. These were causing inconsistent state issues in the pre-0.3.0 provider and are a simple demonstration of why we need the OpenAPI type system.

It would be useful to add assertions for the presence of these two attributes which demonstrate the resource is correctly typed and Terraform saves these computed values as well.

Here an example of how they show up in state:

resource "kubernetes_manifest" "test" {
    manifest = {
        apiVersion = "v1"
        kind       = "Service"
        metadata   = {
            name      = "alextest"
            namespace = "default"
        }
        spec       = {
            ports    = [
                {
                    name       = "http"
                    port       = 80
                    protocol   = "TCP"
                    targetPort = 8080
                },
            ]
            selector = {
                app = "test"
            }
            type     = "ClusterIP"
        }
    }
    object   = {
        apiVersion = "v1"
        kind       = "Service"
        metadata   = {
            annotations                = null
            clusterName                = null
            creationTimestamp          = null
            deletionGracePeriodSeconds = null
            deletionTimestamp          = null
            finalizers                 = null
            generateName               = null
            generation                 = null
            labels                     = null
            managedFields              = null
            name                       = "alextest"
            namespace                  = "default"
            ownerReferences            = null
            resourceVersion            = null
            selfLink                   = null
            uid                        = null
        }
        spec       = {
            allocateLoadBalancerNodePorts = null
            clusterIP                     = "10.104.36.74"
            clusterIPs                    = [
                "10.104.36.74",
            ]
            externalIPs                   = null
            externalName                  = null
            externalTrafficPolicy         = null
            healthCheckNodePort           = null
            ipFamilies                    = null
            ipFamilyPolicy                = null
            loadBalancerIP                = null
            loadBalancerSourceRanges      = null
            ports                         = [
                {
                    appProtocol = null
                    name        = "http"
                    nodePort    = null
                    port        = 80
                    protocol    = "TCP"
                    targetPort  = 8080
                },
            ]
            publishNotReadyAddresses      = null
            selector                      = {
                "app" = "test"
            }
            sessionAffinity               = "None"
            sessionAffinityConfig         = {
                clientIP = {
                    timeoutSeconds = null
                }
            }
            topologyKeys                  = null
            type                          = "ClusterIP"
        }
    }
}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants