diff --git a/docs/configuration-reference/components/velero.md b/docs/configuration-reference/components/velero.md index ad6fc27b6..d078dfd0c 100644 --- a/docs/configuration-reference/components/velero.md +++ b/docs/configuration-reference/components/velero.md @@ -31,64 +31,64 @@ Velero component configuration example: # velero.lokocfg component "velero" { - # provider = "azure/openebs/restic" - # azure { - # # Required arguments. - # subscription_id = "9e5ac23c-6df8-44c4-9790-6f6decf96268" - # tenant_id = "78bdc534-b34f-4bda-a6ca-6df52915b0b5" - # client_id = "d44117a8-b69d-437b-9073-e4e3b25e164a" - # client_secret = "c26f9698-a563-409e-87ee-4dcf96007b73" - # resource_group = "my-resource-group" + # provider = "azure/openebs/restic" + # azure { + # # Required arguments. + # subscription_id = "9e5ac23c-6df8-44c4-9790-6f6decf96268" + # tenant_id = "78bdc534-b34f-4bda-a6ca-6df52915b0b5" + # client_id = "d44117a8-b69d-437b-9073-e4e3b25e164a" + # client_secret = "c26f9698-a563-409e-87ee-4dcf96007b73" + # resource_group = "my-resource-group" # - # backup_storage_location { - # resource_group = "my-resource-group" - # storage_account = "mybackupstorageaccount" - # bucket = "backupscontainer" - # } + # backup_storage_location { + # resource_group = "my-resource-group" + # storage_account = "mybackupstorageaccount" + # bucket = "backupscontainer" + # } # - # # Optional parameters - # volume_snapshot_location { - # resource_group = "my-resource-group" - # api_timeout = "10m" - # } - # } - - # openebs { - # credentials = file("cloud-credentails-file") - # provider = "aws" - # - # backup_storage_location { - # provider = "aws" - # region = "my-region" - # bucket = "my-bucket" - # name = "my-backup-location" - # } + # # Optional parameters + # volume_snapshot_location { + # resource_group = "my-resource-group" + # api_timeout = "10m" + # } + # } # - # volume_snapshot_location { - # bucket = "my-bucket" - # region = "my-region" - # provider = "aws" - # name = "my-snapshot-location" - # prefix = "backup-prefix" - # local = false + # openebs { + # credentials = file("cloud-credentails-file") + # provider = "aws" # - # openebs_namespace = "openebs" + # backup_storage_location { + # provider = "aws" + # region = "my-region" + # bucket = "my-bucket" + # name = "my-backup-location" + # } # - # s3_url = "mybucket.example.com" - # } - # } - - # restic { - # credentials = file("cloud-credentials-file") + # volume_snapshot_location { + # bucket = "my-bucket" + # region = "my-region" + # provider = "aws" + # name = "my-snapshot-location" + # prefix = "backup-prefix" + # local = false + # + # openebs_namespace = "openebs" + # + # s3_url = "mybucket.example.com" + # } + # } + # + # restic { + # credentials = file("cloud-credentials-file") # - # require_volume_annotation = true + # require_volume_annotation = true # - # backup_storage_location { - # provider = "aws" - # bucket = "my-bucket" - # name = "my-backup-location" - # } - # } + # backup_storage_location { + # provider = "aws" + # bucket = "my-bucket" + # name = "my-backup-location" + # } + # } # Optional. metrics { diff --git a/pkg/components/internal/testutil/jsonpath.go b/pkg/components/internal/testutil/jsonpath.go index f72c4eb64..9e3fa2c83 100644 --- a/pkg/components/internal/testutil/jsonpath.go +++ b/pkg/components/internal/testutil/jsonpath.go @@ -89,3 +89,18 @@ func MatchJSONPathStringValue(t *testing.T, yamlConfig string, jsonPath string, t.Fatalf("Expected: %s, Got: %s", expected, got) } } + +// MatchJSONPathInt64Value is a helper function for component unit tests. It compares the integer at +// a JSON path in a YAML config to the expected integer. +func MatchJSONPathInt64Value(t *testing.T, yamlConfig string, jsonPath string, expected int64) { + obj := jsonPathValue(t, yamlConfig, jsonPath) + + got, ok := obj.(int64) + if !ok { + t.Fatalf("Value is not an integer: %#v", obj) + } + + if got != expected { + t.Fatalf("Expected: %d, Got: %d", expected, got) + } +} diff --git a/pkg/components/util/types.go b/pkg/components/util/types.go index bfb201341..0ea5a78f4 100644 --- a/pkg/components/util/types.go +++ b/pkg/components/util/types.go @@ -14,7 +14,10 @@ package util -import "encoding/json" +import ( + "encoding/json" + "fmt" +) // NodeAffinity is a struct that other components can use to define the HCL format of NodeAffinity // in Kubernetes PodSpec. @@ -44,7 +47,7 @@ type Toleration struct { Effect string `hcl:"effect,optional" json:"effect,omitempty"` Operator string `hcl:"operator,optional" json:"operator,omitempty"` Value string `hcl:"value,optional" json:"value,omitempty"` - TolerationSeconds string `hcl:"toleration_seconds,optional" json:"toleration_seconds,omitempty"` + TolerationSeconds int64 `hcl:"toleration_seconds,optional" json:"tolerationSeconds,omitempty"` } // RenderTolerations takes a list of tolerations. @@ -54,6 +57,12 @@ func RenderTolerations(t []Toleration) (string, error) { return "", nil } + for _, toleration := range t { + if err := validateToleration(toleration); err != nil { + return "", fmt.Errorf("toleration validation failed: %v", err) + } + } + b, err := json.Marshal(t) if err != nil { return "", err @@ -62,6 +71,15 @@ func RenderTolerations(t []Toleration) (string, error) { return string(b), nil } +func validateToleration(t Toleration) error { + // If TolerationSeconds is set then; Effect must be `NoExecute` + if t.TolerationSeconds != 0 && t.Effect != "NoExecute" { + return fmt.Errorf("`effect` must be `NoExecute` as `toleration_seconds` is set: got %s", t.Effect) + } + + return nil +} + // NodeSelector is a type used when defining node selector for the pod spec. type NodeSelector map[string]string diff --git a/pkg/components/util/types_test.go b/pkg/components/util/types_test.go new file mode 100644 index 000000000..1cce161b1 --- /dev/null +++ b/pkg/components/util/types_test.go @@ -0,0 +1,47 @@ +// Copyright 2021 The Lokomotive Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package util_test + +import ( + "testing" + + "github.com/kinvolk/lokomotive/pkg/components/util" +) + +func TestTolerationValidation(t *testing.T) { + testCases := []struct { + desc string + toleration util.Toleration + wantErr bool + }{ + { + desc: "TolerationSeconds set; Effect must be `NoExecute`", + toleration: util.Toleration{ + Key: "testkey", + Value: "testvalue", + Effect: "NoSchedule", + TolerationSeconds: 3, + }, + wantErr: true, + }, + } + + for _, tc := range testCases { + _, err := util.RenderTolerations(append([]util.Toleration{}, tc.toleration)) + if !tc.wantErr && err != nil { + t.Fatalf("Valid Toleration should not return error, got: %s", err) + } + } +} diff --git a/pkg/components/velero/component_test.go b/pkg/components/velero/component_test.go index a928da68c..47e8ca85f 100644 --- a/pkg/components/velero/component_test.go +++ b/pkg/components/velero/component_test.go @@ -170,6 +170,7 @@ component "velero" { backup_storage_location { bucket = "foo" provider = "aws" + region = "myregion" } } } @@ -207,14 +208,14 @@ component "velero" { backup_storage_location { bucket = "foo" provider = "aws" + region = "myregion" } - tolerations { - key = "TestResticToletrationKey" - value = "TestResticToletrationValue" + key = "TestResticTolerationKey" + value = "TestResticTolerationValue" operator = "Equal" - effect = "NoSchedule" - toleration_seconds = "1" + effect = "NoExecute" + toleration_seconds = 1 } } } @@ -233,9 +234,16 @@ component "velero" { m := testutil.RenderManifests(t, component, Name, configHCL) jsonPath := "{.spec.template.spec.tolerations[0].key}" - expected := "TestResticToletrationKey" + expected := "TestResticTolerationKey" gotConfig := testutil.ConfigFromMap(t, m, "velero/templates/restic-daemonset.yaml") testutil.MatchJSONPathStringValue(t, gotConfig, jsonPath, expected) + + jsonPath = "{.spec.template.spec.tolerations[0].tolerationSeconds}" + expectedNumber := int64(1) + + gotConfig = testutil.ConfigFromMap(t, m, "velero/templates/restic-daemonset.yaml") + + testutil.MatchJSONPathInt64Value(t, gotConfig, jsonPath, expectedNumber) } diff --git a/pkg/components/velero/restic/restic.go b/pkg/components/velero/restic/restic.go index 38491d112..b84afdba9 100644 --- a/pkg/components/velero/restic/restic.go +++ b/pkg/components/velero/restic/restic.go @@ -111,6 +111,14 @@ func (c *Configuration) Validate() hcl.Diagnostics { }) } + if c.BackupStorageLocation.Provider == "aws" && c.BackupStorageLocation.Region == "" { + diagnostics = append(diagnostics, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Make sure `restic.backup_storage_location.region` value is set", + Detail: "restic.backup_storage_location.region must be provided for `aws` provider", + }) + } + return diagnostics } diff --git a/pkg/components/velero/restic/restic_test.go b/pkg/components/velero/restic/restic_test.go new file mode 100644 index 000000000..6695dbb5f --- /dev/null +++ b/pkg/components/velero/restic/restic_test.go @@ -0,0 +1,71 @@ +// Copyright 2021 The Lokomotive Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package restic_test + +import ( + "testing" + + "github.com/kinvolk/lokomotive/pkg/components/util" + "github.com/kinvolk/lokomotive/pkg/components/velero/restic" +) + +//nolint:funlen +func TestResticConfig(t *testing.T) { + tests := []struct { + desc string + config *restic.Configuration + wantError bool + }{ + { + desc: "effect should be NoExecute if TolerationSeconds is Set", + wantError: true, + config: &restic.Configuration{ + Credentials: "foo", + BackupStorageLocation: &restic.BackupStorageLocation{ + Bucket: "mybucket", + Provider: "aws", + Region: "myregion", + }, + Tolerations: []util.Toleration{ + { + Key: "key", + Value: "value", + Effect: "NoSchedule", + Operator: "Equal", + TolerationSeconds: 3, + }, + }, + }, + }, + { + desc: "region should be present if provider is aws", + wantError: true, + config: &restic.Configuration{ + Credentials: "foo", + BackupStorageLocation: &restic.BackupStorageLocation{ + Bucket: "mybucket", + Provider: "aws", + }, + }, + }, + } + + for _, tc := range tests { + diags := tc.config.Validate() + if !tc.wantError && diags.HasErrors() { + t.Fatalf("Valid config should not return error, got: %s", diags.Error()) + } + } +}