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.

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

fix: jaas integration test #641

Merged
merged 3 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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: 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]
alesstimec marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I remember I investigated moving to self hosted runners and it didn't work because docker compose wasn't available. I asked about adding it to self-hosted runners and they said they'd add it to their backlog (this was ~3 months ago).
But I see you're already installing it all yourself below which works too.

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
Copy link
Member

@anvial anvial Dec 13, 2024

Choose a reason for hiding this comment

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

I would say, that this 4 line should be just deleted.

          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

Because for test we need only microk8s-config.yaml exists. We do not require ENVAR anymore.

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 in case adding k8s cluster from existed 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 in case adding k8s cluster from existed 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
63 changes: 57 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 in case adding k8s cluster from existed 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.",
alesstimec marked this conversation as resolved.
Show resolved Hide resolved
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 in case adding k8s cluster from existed 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.",
alesstimec marked this conversation as resolved.
Show resolved Hide resolved
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,45 @@ 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 this resource can only be used with JAAS"
}

// 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", "parent_cloud_name must be specified when applying to a JAAS controller")
alesstimec marked this conversation as resolved.
Show resolved Hide resolved
}

if data.ParentCloudRegion.ValueString() == "" {
resp.Diagnostics.AddError("Plan Error", "parent_cloud_region must be specified when applying to a JAAS controller")
alesstimec marked this conversation as resolved.
Show resolved Hide resolved
}
}

func newKubernetesCloudID(kubernetesCloudName string, cloudCredentialName string) string {
return fmt.Sprintf("%s:%s", kubernetesCloudName, cloudCredentialName)
}
53 changes: 51 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,56 @@ 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: testAccResourceKubernetesCloudWithoutParentCloud(cloudName, cloudConfig),
ExpectError: regexp.MustCompile("parent_cloud_region must be specified when applying to a JAAS controller"),
},
},
})
}

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 testAccResourceKubernetesCloudWithoutParentCloud(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
Loading