Skip to content

Commit

Permalink
Merge pull request #641 from alesstimec/jaas-integration-fix
Browse files Browse the repository at this point in the history
fix: jaas integration test
  • Loading branch information
alesstimec authored Dec 19, 2024
2 parents d8077ec + d344dfa commit fbd1ccc
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 19 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/test_integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ jobs:
TF_ACC: "1"
TEST_CLOUD: ${{ matrix.action-operator.cloud }}
run: go test -parallel 1 -timeout 60m -v -cover ./internal/provider/
timeout-minutes: 40
timeout-minutes: 60

# Run acceptance tests in a matrix with Terraform CLI versions
add-machine-test:
Expand Down Expand Up @@ -189,5 +189,5 @@ jobs:
echo "Running the test"
cd ./internal/provider/
go test ./... -timeout 30m -v -test.run TestAcc_ResourceMachine_AddMachine
timeout-minutes: 40
go test ./... -timeout 60m -v -test.run TestAcc_ResourceMachine_AddMachine
timeout-minutes: 60
19 changes: 13 additions & 6 deletions .github/workflows/test_integration_jaas.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ jobs:
test:
name: Integration-JAAS
needs: build
runs-on: ubuntu-latest
runs-on: [self-hosted, jammy, x64]
strategy:
fail-fast: false
timeout-minutes: 60
Expand All @@ -55,6 +55,10 @@ jobs:
with:
terraform_version: "1.9.*"
terraform_wrapper: false
- name: Install docker compose plugin
run: |
for pkg in docker.io docker-doc docker-compose docker-compose-v2 podman-docker containerd runc; do sudo apt-get remove -y $pkg; done
sudo snap install docker --channel latest/stable
# Starting JAAS will start the JIMM controller and dependencies and create a Juju controller on LXD and connect it to JIMM.
- name: Setup JAAS
uses: canonical/jimm/.github/actions/test-server@v3
Expand All @@ -68,11 +72,13 @@ jobs:
sudo snap install microk8s --channel=1.28-strict/stable
sudo usermod -a -G snap_microk8s $USER
sudo chown -R $USER ~/.kube
sudo microk8s.enable dns storage
sudo microk8s.enable dns local-storage
sudo microk8s.enable dns
sudo microk8s.enable storage
sudo microk8s.enable hostpath-storage
sudo -g snap_microk8s -E microk8s status --wait-ready --timeout=600
sudo microk8s.config view | tee /home/$USER/microk8s-config.yaml
echo "MICROK8S_CONFIG<<EOF" >> $GITHUB_ENV
sudo microk8s.config view >> $GITHUB_ENV
echo "$(cat /home/${USER}/microk8s-config.yaml)" >> $GITHUB_ENV
echo "EOF" >> $GITHUB_ENV
- name: Create additional networks when testing with LXD
run: |
Expand All @@ -97,5 +103,6 @@ jobs:
- env:
TF_ACC: "1"
TEST_CLOUD: "lxd"
run: go test -parallel 10 -timeout 40m -v -cover ./internal/provider/
timeout-minutes: 40
run: go test -parallel 1 -timeout 60m -v -cover ./internal/provider/
timeout-minutes: 60

4 changes: 2 additions & 2 deletions docs/resources/kubernetes_cloud.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ resource "juju_model" "my-model" {
### Optional

- `kubernetes_config` (String, Sensitive) The kubernetes config file path for the cloud. Cloud credentials will be added to the Juju controller for you.
- `parent_cloud_name` (String) The parent cloud name in case adding k8s cluster from existed cloud. Changing this value will cause the cloud to be destroyed and recreated by terraform.
- `parent_cloud_region` (String) The parent cloud region name in case adding k8s cluster from existed cloud. Changing this value will cause the cloud to be destroyed and recreated by terraform.
- `parent_cloud_name` (String) The parent cloud name, for adding a k8s cluster from an existing cloud. Changing this value will cause the cloud to be destroyed and recreated by terraform. *Note* that this value must be set when running against a JAAS controller.
- `parent_cloud_region` (String) The parent cloud region name, for adding a k8s cluster from an existing cloud. Changing this value will cause the cloud to be destroyed and recreated by terraform. *Note* that this value must be set when running against a JAAS controller.

### Read-Only

Expand Down
59 changes: 53 additions & 6 deletions internal/provider/resource_kubernetes_cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
// Ensure provider defined types fully satisfy framework interfaces.
var _ resource.Resource = &kubernetesCloudResource{}
var _ resource.ResourceWithConfigure = &kubernetesCloudResource{}
var _ resource.ResourceWithConfigValidators = &kubernetesCloudResource{}

func NewKubernetesCloudResource() resource.Resource {
return &kubernetesCloudResource{}
Expand Down Expand Up @@ -62,6 +63,13 @@ func (r *kubernetesCloudResource) Configure(ctx context.Context, req resource.Co
r.subCtx = tflog.NewSubsystem(ctx, LogResourceKubernetesCloud)
}

// ConfigValidators returns a list of functions which will all be performed during validation.
func (r *kubernetesCloudResource) ConfigValidators(context.Context) []resource.ConfigValidator {
return []resource.ConfigValidator{
&kuberenetesCloudJAASValidator{r.client},
}
}

// Metadata returns the metadata for the kubernetes cloud resource.
func (r *kubernetesCloudResource) Metadata(_ context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) {
resp.TypeName = req.ProviderTypeName + "_kubernetes_cloud"
Expand Down Expand Up @@ -92,11 +100,11 @@ func (r *kubernetesCloudResource) Schema(_ context.Context, req resource.SchemaR
Sensitive: true,
},
"parent_cloud_name": schema.StringAttribute{
Description: "The parent cloud name in case adding k8s cluster from existed cloud. Changing this value will cause the cloud to be destroyed and recreated by terraform.",
Description: "The parent cloud name, for adding a k8s cluster from an existing cloud. Changing this value will cause the cloud to be destroyed and recreated by terraform. *Note* that this value must be set when running against a JAAS controller.",
Optional: true,
},
"parent_cloud_region": schema.StringAttribute{
Description: "The parent cloud region name in case adding k8s cluster from existed cloud. Changing this value will cause the cloud to be destroyed and recreated by terraform.",
Description: "The parent cloud region name, for adding a k8s cluster from an existing cloud. Changing this value will cause the cloud to be destroyed and recreated by terraform. *Note* that this value must be set when running against a JAAS controller.",
Optional: true,
},
"id": schema.StringAttribute{
Expand Down Expand Up @@ -128,8 +136,10 @@ func (r *kubernetesCloudResource) Create(ctx context.Context, req resource.Creat
// Create the kubernetes cloud.
cloudCredentialName, err := r.client.Clouds.CreateKubernetesCloud(
&juju.CreateKubernetesCloudInput{
Name: plan.CloudName.ValueString(),
KubernetesConfig: plan.KubernetesConfig.ValueString(),
Name: plan.CloudName.ValueString(),
KubernetesConfig: plan.KubernetesConfig.ValueString(),
ParentCloudName: plan.ParentCloudName.ValueString(),
ParentCloudRegion: plan.ParentCloudRegion.ValueString(),
},
)
if err != nil {
Expand Down Expand Up @@ -201,8 +211,10 @@ func (r *kubernetesCloudResource) Update(ctx context.Context, req resource.Updat
// Update the kubernetes cloud.
err := r.client.Clouds.UpdateKubernetesCloud(
juju.UpdateKubernetesCloudInput{
Name: plan.CloudName.ValueString(),
KubernetesConfig: plan.KubernetesConfig.ValueString(),
Name: plan.CloudName.ValueString(),
KubernetesConfig: plan.KubernetesConfig.ValueString(),
ParentCloudName: plan.ParentCloudName.ValueString(),
ParentCloudRegion: plan.ParentCloudRegion.ValueString(),
},
)
if err != nil {
Expand Down Expand Up @@ -250,6 +262,41 @@ func (r *kubernetesCloudResource) trace(msg string, additionalFields ...map[stri
tflog.SubsystemTrace(r.subCtx, LogResourceKubernetesCloud, msg, additionalFields...)
}

type kuberenetesCloudJAASValidator struct {
client *juju.Client
}

// Description implements the Description method of the resource.ConfigValidator interface.
func (v *kuberenetesCloudJAASValidator) Description(ctx context.Context) string {
return v.MarkdownDescription(ctx)
}

// MarkdownDescription implements the MarkdownDescription method of the resource.ConfigValidator interface.
func (v *kuberenetesCloudJAASValidator) MarkdownDescription(_ context.Context) string {
return "Enforces that the parent_cloud_name is specified when applying to a JAAS controller."
}

// ValidateResource implements the ValidateResource method of the resource.ConfigValidator interface.
func (v *kuberenetesCloudJAASValidator) ValidateResource(ctx context.Context, req resource.ValidateConfigRequest, resp *resource.ValidateConfigResponse) {
if v.client == nil {
return
}

if !v.client.IsJAAS() {
return
}

var data kubernetesCloudResourceModel
resp.Diagnostics.Append(req.Config.Get(ctx, &data)...)
if resp.Diagnostics.HasError() {
return
}

if data.ParentCloudName.ValueString() == "" {
resp.Diagnostics.AddError("Plan Error", "Field `parent_cloud_name` must be specified when applying to a JAAS controller.")
}
}

func newKubernetesCloudID(kubernetesCloudName string, cloudCredentialName string) string {
return fmt.Sprintf("%s:%s", kubernetesCloudName, cloudCredentialName)
}
51 changes: 49 additions & 2 deletions internal/provider/resource_kubernetes_cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package provider

import (
"os"
"regexp"
"testing"

"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
Expand All @@ -24,12 +25,17 @@ func TestAcc_ResourceKubernetesCloud(t *testing.T) {
cloudName := acctest.RandomWithPrefix("tf-test-k8scloud")
cloudConfig := os.Getenv("MICROK8S_CONFIG")

jaasTest := false
if _, ok := os.LookupEnv("IS_JAAS"); ok {
jaasTest = true
}

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ProtoV6ProviderFactories: frameworkProviderFactories,
Steps: []resource.TestStep{
{
Config: testAccResourceKubernetesCloudWithoutModel(cloudName, cloudConfig),
Config: testAccResourceKubernetesCloudWithoutModel(cloudName, cloudConfig, jaasTest),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("juju_kubernetes_cloud."+cloudName, "name", cloudName),
),
Expand All @@ -38,13 +44,54 @@ func TestAcc_ResourceKubernetesCloud(t *testing.T) {
})
}

func testAccResourceKubernetesCloudWithoutModel(cloudName string, config string) string {
func TestAcc_ResourceKubernetesCloudWithJAASIncompleteConfig(t *testing.T) {
OnlyTestAgainstJAAS(t)
// TODO: This test is not adding model as a resource, which is required.
// The reason in the race that we (potentially) have in the Juju side.
// Once the problem is fixed (https://bugs.launchpad.net/juju/+bug/2084448),
// we should add the model as a resource.
if testingCloud != LXDCloudTesting {
t.Skip(t.Name() + " only runs with LXD")
}
cloudName := acctest.RandomWithPrefix("tf-test-k8scloud")
cloudConfig := os.Getenv("MICROK8S_CONFIG")

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ProtoV6ProviderFactories: frameworkProviderFactories,
Steps: []resource.TestStep{{
Config: testAccResourceKubernetesCloudWithoutParentCloudName(cloudName, cloudConfig),
ExpectError: regexp.MustCompile("Field `parent_cloud_name` must be specified when applying to a JAAS.*"),
}},
})
}

func testAccResourceKubernetesCloudWithoutModel(cloudName string, config string, jaasTest bool) string {
return internaltesting.GetStringFromTemplateWithData(
"testAccResourceSecret",
`
resource "juju_kubernetes_cloud" "{{.CloudName}}" {
name = "{{.CloudName}}"
kubernetes_config = file("~/microk8s-config.yaml")
{{ if .JAASTest }}
parent_cloud_name = "lxd"
parent_cloud_region = "localhost"
{{ end }}
}
`, internaltesting.TemplateData{
"CloudName": cloudName,
"Config": config,
"JAASTest": jaasTest,
})
}

func testAccResourceKubernetesCloudWithoutParentCloudName(cloudName string, config string) string {
return internaltesting.GetStringFromTemplateWithData(
"testAccResourceSecret",
`
resource "juju_kubernetes_cloud" "{{.CloudName}}" {
name = "{{.CloudName}}"
kubernetes_config = "test config"
}
`, internaltesting.TemplateData{
"CloudName": cloudName,
Expand Down

0 comments on commit fbd1ccc

Please sign in to comment.