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

Add divisor to resource_field_ref #1063

Merged
merged 3 commits into from
Nov 18, 2020
Merged

Add divisor to resource_field_ref #1063

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6eba3ab
Add divisor to resource_field_ref
dak1n1 Nov 11, 2020
a246cf8
Update wait_for logic to wait until pods are Ready
dak1n1 Nov 12, 2020
5f9a81c
fix test
dak1n1 Nov 12, 2020
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
6 changes: 5 additions & 1 deletion kubernetes/resource_kubernetes_deployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ func waitForDeploymentReplicasFunc(ctx context.Context, conn *kubernetes.Clients
return resource.NonRetryableError(err)
}

var specReplicas int32 = 1 // default, acording to API docs
var specReplicas int32 = 1 // default, according to API docs
if dply.Spec.Replicas != nil {
specReplicas = *dply.Spec.Replicas
}
Expand All @@ -436,6 +436,10 @@ func waitForDeploymentReplicasFunc(ctx context.Context, conn *kubernetes.Clients
return resource.RetryableError(fmt.Errorf("Waiting for rollout to finish: %d old replicas are pending termination...", dply.Status.Replicas-dply.Status.UpdatedReplicas))
}

if dply.Status.Replicas > dply.Status.ReadyReplicas {
return resource.RetryableError(fmt.Errorf("Waiting for rollout to finish: %d replicas wanted; %d replicas Ready", dply.Status.Replicas, dply.Status.ReadyReplicas))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This piece of logic was originally inside an acceptance test, and it would fail periodically because the deployment wasn't finished rolling out. So I added it into wait_for_rollout, and now it waits until the deployment says all pods are ready.

}

if dply.Status.AvailableReplicas < dply.Status.UpdatedReplicas {
return resource.RetryableError(fmt.Errorf("Waiting for rollout to finish: %d of %d updated replicas are available...", dply.Status.AvailableReplicas, dply.Status.UpdatedReplicas))
}
Expand Down
179 changes: 127 additions & 52 deletions kubernetes/resource_kubernetes_deployment_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ func TestAccKubernetesDeployment_basic(t *testing.T) {
Config: testAccKubernetesDeploymentConfig_basic(name),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckKubernetesDeploymentExists("kubernetes_deployment.test", &conf),
testAccCheckKubernetesDeploymentRolledOut("kubernetes_deployment.test"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "metadata.0.annotations.%", "2"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "metadata.0.annotations.TestAnnotationOne", "one"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "metadata.0.annotations.TestAnnotationTwo", "two"),
Expand Down Expand Up @@ -436,24 +435,15 @@ func TestAccKubernetesDeployment_with_container_security_context_run_as_group(t
testAccCheckKubernetesDeploymentExists("kubernetes_deployment.test", &conf),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.#", "2"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.0.security_context.#", "1"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.0.security_context.0.capabilities.#", "0"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.0.security_context.0.privileged", "true"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.0.security_context.0.se_linux_options.#", "1"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.0.security_context.0.se_linux_options.0.level", "s0:c123,c456"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.1.security_context.#", "1"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.1.security_context.0.allow_privilege_escalation", "true"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.1.security_context.0.capabilities.#", "1"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.1.security_context.0.capabilities.0.add.#", "1"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.1.security_context.0.capabilities.0.add.0", "NET_BIND_SERVICE"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.1.security_context.0.capabilities.0.drop.#", "1"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.1.security_context.0.capabilities.0.drop.0", "all"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.1.security_context.0.privileged", "true"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.1.security_context.0.read_only_root_filesystem", "true"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.1.security_context.0.privileged", "false"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.1.security_context.0.run_as_group", "200"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.1.security_context.0.run_as_non_root", "true"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.1.security_context.0.run_as_non_root", "false"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.1.security_context.0.run_as_user", "201"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.1.security_context.0.se_linux_options.#", "1"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.1.security_context.0.se_linux_options.0.level", "s0:c123,c789"),
),
},
},
Expand Down Expand Up @@ -915,14 +905,14 @@ func TestAccKubernetesDeployment_regression(t *testing.T) {
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
IDRefreshName: "kubernetes_deployment.test",
IDRefreshIgnore: []string{"metadata.0.resource_version"},
ExternalProviders: testAccExternalProviders,
CheckDestroy: testAccCheckKubernetesDeploymentDestroy,
Steps: []resource.TestStep{
{
Config: requiredProviders() + testAccKubernetesDeploymentConfig_regression("kubernetes-released", name),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckKubernetesDeploymentExists("kubernetes_deployment.test", &conf1),
testAccCheckKubernetesDeploymentRolledOut("kubernetes_deployment.test"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "metadata.0.annotations.%", "2"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "metadata.0.annotations.TestAnnotationOne", "one"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "metadata.0.annotations.TestAnnotationTwo", "two"),
Expand All @@ -938,7 +928,7 @@ func TestAccKubernetesDeployment_regression(t *testing.T) {
resource.TestCheckResourceAttrSet("kubernetes_deployment.test", "metadata.0.self_link"),
resource.TestCheckResourceAttrSet("kubernetes_deployment.test", "metadata.0.uid"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.0.image", defaultNginxImage),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.0.name", "tf-acc-test"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.0.name", "containername"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.strategy.0.type", "RollingUpdate"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.strategy.0.rolling_update.0.max_surge", "25%"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.strategy.0.rolling_update.0.max_unavailable", "25%"),
Expand All @@ -949,7 +939,6 @@ func TestAccKubernetesDeployment_regression(t *testing.T) {
Config: requiredProviders() + testAccKubernetesDeploymentConfig_regression("kubernetes-local", name),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckKubernetesDeploymentExists("kubernetes_deployment.test", &conf2),
testAccCheckKubernetesDeploymentRolledOut("kubernetes_deployment.test"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "metadata.0.annotations.%", "2"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "metadata.0.annotations.TestAnnotationOne", "one"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "metadata.0.annotations.TestAnnotationTwo", "two"),
Expand All @@ -965,7 +954,7 @@ func TestAccKubernetesDeployment_regression(t *testing.T) {
resource.TestCheckResourceAttrSet("kubernetes_deployment.test", "metadata.0.self_link"),
resource.TestCheckResourceAttrSet("kubernetes_deployment.test", "metadata.0.uid"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.0.image", defaultNginxImage),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.0.name", "tf-acc-test"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.0.name", "containername"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.strategy.0.type", "RollingUpdate"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.strategy.0.rolling_update.0.max_surge", "25%"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.strategy.0.rolling_update.0.max_unavailable", "25%"),
Expand All @@ -977,6 +966,57 @@ func TestAccKubernetesDeployment_regression(t *testing.T) {
})
}

func TestAccKubernetesDeployment_with_resource_field_selector(t *testing.T) {
var conf appsv1.Deployment
rcName := fmt.Sprintf("tf-acc-test-%s", acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum))
imageName := "nginx:1.7.9"

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ProviderFactories: testAccProviderFactories,
CheckDestroy: testAccCheckKubernetesDeploymentDestroy,
Steps: []resource.TestStep{
{
ExpectError: regexp.MustCompile("quantities must match the regular expression"),
Config: testAccKubernetesDeploymentConfigWithResourceFieldSelector(rcName, imageName, "limits.cpu", ""),
},
{
ExpectError: regexp.MustCompile("only divisor's values 1m and 1 are supported with the cpu resource"),
Config: testAccKubernetesDeploymentConfigWithResourceFieldSelector(rcName, imageName, "limits.cpu", "2"),
},
{
Config: testAccKubernetesDeploymentConfigWithResourceFieldSelector(rcName, imageName, "limits.cpu", "1m"),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckKubernetesDeploymentExists("kubernetes_deployment.test", &conf),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.0.image", imageName),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.0.resources.0.limits.0.memory", "512Mi"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.0.env.#", "1"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.0.env.0.name", "K8S_LIMITS_CPU"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.0.env.0.value_from.0.resource_field_ref.0.container_name", "containername"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.0.env.0.value_from.0.resource_field_ref.0.divisor", "1m"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.0.env.0.value_from.0.resource_field_ref.0.resource", "limits.cpu"),
),
},
{
Config: testAccKubernetesDeploymentConfigWithResourceFieldSelector(rcName, imageName, "limits.memory", "1Mi"),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckKubernetesDeploymentExists("kubernetes_deployment.test", &conf),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.0.env.0.value_from.0.resource_field_ref.0.divisor", "1Mi"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.0.env.0.value_from.0.resource_field_ref.0.resource", "limits.memory"),
),
},
{
Config: testAccKubernetesDeploymentConfigWithResourceFieldSelector(rcName, imageName, "requests.memory", "1Ki"),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckKubernetesDeploymentExists("kubernetes_deployment.test", &conf),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.0.env.0.value_from.0.resource_field_ref.0.divisor", "1Ki"),
resource.TestCheckResourceAttr("kubernetes_deployment.test", "spec.0.template.0.spec.0.container.0.env.0.value_from.0.resource_field_ref.0.resource", "requests.memory"),
),
},
},
})
}

func testAccCheckKubernetesDeploymentForceNew(old, new *appsv1.Deployment, wantNew bool) resource.TestCheckFunc {
return func(s *terraform.State) error {
if wantNew {
Expand Down Expand Up @@ -1095,21 +1135,6 @@ func testAccCheckKubernetesDeploymentRollingOut(n string) resource.TestCheckFunc
}
}

func testAccCheckKubernetesDeploymentRolledOut(n string) resource.TestCheckFunc {
return func(s *terraform.State) error {
d, err := getDeploymentFromResourceName(s, n)
if err != nil {
return err
}

if d.Status.Replicas != d.Status.ReadyReplicas {
return fmt.Errorf("deployment is still rolling out")
}

return nil
}
}

func testAccKubernetesDeploymentConfig_basic(name string) string {
return fmt.Sprintf(`resource "kubernetes_deployment" "test" {
metadata {
Expand Down Expand Up @@ -1841,27 +1866,12 @@ func testAccKubernetesDeploymentConfigWithContainerSecurityContextRunAsGroup(dep
}

container {
image = "gcr.io/google_containers/liveness"
name = "containername2"
args = ["/server"]

name = "container2"
image = "busybox"
command = ["sh", "-c", "echo The app is running! && sleep 3600"]
security_context {
allow_privilege_escalation = true

capabilities {
drop = ["all"]
add = ["NET_BIND_SERVICE"]
}

privileged = true
read_only_root_filesystem = true
run_as_group = 200
run_as_non_root = true
run_as_user = 201

se_linux_options {
level = "s0:c123,c789"
}
run_as_group = 200
run_as_user = 201
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test had been failing for a long time, due to some permissions issues that I think were related to the container image. This change gives us a working container and still tests the run_as_group.

}
}
Expand Down Expand Up @@ -2388,7 +2398,7 @@ func testAccKubernetesDeploymentConfig_regression(provider, name string) string
spec {
container {
image = %q
name = "tf-acc-test"
name = "containername"

port {
container_port = 80
Expand All @@ -2408,6 +2418,24 @@ func testAccKubernetesDeploymentConfig_regression(provider, name string) string
cpu = "50m"
}
}
env {
name = "LIMITS_CPU"
value_from {
resource_field_ref {
container_name = "containername"
resource = "requests.cpu"
}
}
}
env {
name = "LIMITS_MEM"
value_from {
resource_field_ref {
container_name = "containername"
resource = "requests.memory"
Copy link
Contributor Author

@dak1n1 dak1n1 Nov 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I added this section to the regression test to ensure that it wasn't going to overwrite existing deployments that use resource_field_ref. (divisor is intentionally omitted, because it doesn't exist in the released version of the kubernetes provider, so I can't test it here yet).

}
}
}
}
}
}
Expand Down Expand Up @@ -2474,3 +2502,50 @@ resource "kubernetes_deployment" "test" {
}
`, secretName, label, deploymentName, imageName)
}

func testAccKubernetesDeploymentConfigWithResourceFieldSelector(rcName, imageName, resourceName, divisor string) string {
return fmt.Sprintf(`resource "kubernetes_deployment" "test" {
metadata {
name = "%s"
labels = {
Test = "TfAcceptanceTest"
}
}
spec {
selector {
match_labels = {
Test = "TfAcceptanceTest"
}
}
template {
metadata {
labels = {
Test = "TfAcceptanceTest"
}
}
spec {
container {
image = "%s"
name = "containername"
resources {
limits {
memory = "512Mi"
}
}
env {
name = "K8S_LIMITS_CPU"
value_from {
resource_field_ref {
container_name = "containername"
resource = "%s"
divisor = "%s"
}
}
}
}
}
}
}
}
`, rcName, imageName, resourceName, divisor)
}
7 changes: 7 additions & 0 deletions kubernetes/schema_container.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,13 @@ func containerFields(isUpdatable, isInitContainer bool) map[string]*schema.Schem
Type: schema.TypeString,
Optional: true,
},
"divisor": {
Type: schema.TypeString,
Optional: true,
Default: "1",
ValidateFunc: validateResourceQuantity,
DiffSuppressFunc: suppressEquivalentResourceQuantity,
},
"resource": {
Type: schema.TypeString,
Required: true,
Expand Down
9 changes: 6 additions & 3 deletions kubernetes/schema_pod_spec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -547,9 +547,12 @@ func volumeSchema(isUpdatable bool) *schema.Resource {
Type: schema.TypeString,
Required: true,
},
"quantity": {
Type: schema.TypeString,
Optional: true,
"divisor": {
Type: schema.TypeString,
Optional: true,
Default: "1",
ValidateFunc: validateResourceQuantity,
DiffSuppressFunc: suppressEquivalentResourceQuantity,
},
"resource": {
Type: schema.TypeString,
Expand Down
13 changes: 12 additions & 1 deletion kubernetes/structures_container.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
package kubernetes

import (
v1 "k8s.io/api/core/v1"
"k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/apimachinery/pkg/util/intstr"
)

Expand Down Expand Up @@ -205,6 +206,9 @@ func flattenResourceFieldSelector(in *v1.ResourceFieldSelector) []interface{} {
if in.Resource != "" {
att["resource"] = in.Resource
}
if in.Divisor.String() != "" {
att["divisor"] = in.Divisor.String()
}
return []interface{}{att}
}

Expand Down Expand Up @@ -861,6 +865,13 @@ func expandResourceFieldRef(r []interface{}) (*v1.ResourceFieldSelector, error)
if v, ok := in["resource"].(string); ok {
obj.Resource = v
}
if v, ok := in["divisor"].(string); ok {
q, err := resource.ParseQuantity(v)
if err != nil {
return obj, err
}
obj.Divisor = q
}
return obj, nil
}

Expand Down