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 1 commit
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
18 changes: 12 additions & 6 deletions .github/workflows/test_integration_jaas.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ jobs:
# Ensure project builds before running test
build:
name: Build-JAAS
runs-on: ubuntu-latest
runs-on: [self-hosted, jammy, x64]
Copy link
Member

Choose a reason for hiding this comment

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

The build jobs should all run on GitHub runners, not self-hosted. They are too short and will delay the over all time.

timeout-minutes: 5
steps:
- uses: actions/checkout@v4
Expand All @@ -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,5 @@ jobs:
- env:
TF_ACC: "1"
TEST_CLOUD: "lxd"
run: go test -parallel 10 -timeout 40m -v -cover ./internal/provider/
run: go test -parallel 1 -timeout 40m -v -cover ./internal/provider/
timeout-minutes: 40
6 changes: 6 additions & 0 deletions internal/provider/resource_kubernetes_cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ import (
)

func TestAcc_ResourceKubernetesCloud(t *testing.T) {
// Note (alesstimec): Skipping this test, because the default
// hosted cloud tf provider adds is "other", which cannot
// be parsed by JIMM - it needs a valid cloud/region to determine
// which controller to add the cloud to.
SkipJAAS(t)
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to install and add the microk8s config to the JAAS integration tests environment if there are no tests that are using it?


// 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),
Expand Down
Loading