From 0df12a0f12889603d644e557d226f8da2ef90dea Mon Sep 17 00:00:00 2001 From: Adam Dyess Date: Mon, 6 May 2024 15:53:59 -0500 Subject: [PATCH 1/2] pin operator-workflow to a commit (#100) * pin operator-workflow to a commit * No long raise_on_blocked, worker charms will be blocked by default until the unit processes the join cluster relation hook --- .github/workflows/auto-update-libs-k8s-worker.yaml | 2 +- .github/workflows/charm-analysis.yaml | 2 +- .github/workflows/comment.yaml | 2 +- .github/workflows/integration_test.yaml | 2 +- .github/workflows/load_test.yaml | 2 +- .github/workflows/promote-charms.yaml | 2 +- .github/workflows/publish-charms.yaml | 2 +- tests/integration/conftest.py | 13 +------------ tests/integration/test_etcd.py | 1 - 9 files changed, 8 insertions(+), 20 deletions(-) diff --git a/.github/workflows/auto-update-libs-k8s-worker.yaml b/.github/workflows/auto-update-libs-k8s-worker.yaml index 79dd44fa..a1c8e634 100644 --- a/.github/workflows/auto-update-libs-k8s-worker.yaml +++ b/.github/workflows/auto-update-libs-k8s-worker.yaml @@ -6,7 +6,7 @@ on: jobs: auto-update-libs: - uses: canonical/operator-workflows/.github/workflows/auto_update_charm_libs.yaml@main + uses: canonical/operator-workflows/.github/workflows/auto_update_charm_libs.yaml@08c5a65a0bc4696164b4f85a29a9ccbd830d10d8 secrets: inherit with: working-directory: ./charms/worker/k8s diff --git a/.github/workflows/charm-analysis.yaml b/.github/workflows/charm-analysis.yaml index b064a784..e4b8f0fa 100644 --- a/.github/workflows/charm-analysis.yaml +++ b/.github/workflows/charm-analysis.yaml @@ -6,7 +6,7 @@ on: jobs: unit-tests: - uses: canonical/operator-workflows/.github/workflows/test.yaml@main + uses: canonical/operator-workflows/.github/workflows/test.yaml@08c5a65a0bc4696164b4f85a29a9ccbd830d10d8 secrets: inherit with: charm-directory: charms diff --git a/.github/workflows/comment.yaml b/.github/workflows/comment.yaml index 26ac226d..eb8379ce 100644 --- a/.github/workflows/comment.yaml +++ b/.github/workflows/comment.yaml @@ -8,5 +8,5 @@ on: jobs: comment-on-pr: - uses: canonical/operator-workflows/.github/workflows/comment.yaml@main + uses: canonical/operator-workflows/.github/workflows/comment.yaml@08c5a65a0bc4696164b4f85a29a9ccbd830d10d8 secrets: inherit diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index 6e898deb..d092b757 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -36,7 +36,7 @@ jobs: working-directory: ${{ matrix.path }} integration-tests: - uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main + uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@08c5a65a0bc4696164b4f85a29a9ccbd830d10d8 needs: [build-all-charms, extra-args] strategy: matrix: diff --git a/.github/workflows/load_test.yaml b/.github/workflows/load_test.yaml index c0c130e8..792599fb 100644 --- a/.github/workflows/load_test.yaml +++ b/.github/workflows/load_test.yaml @@ -6,7 +6,7 @@ on: jobs: load-tests: - uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main + uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@08c5a65a0bc4696164b4f85a29a9ccbd830d10d8 with: provider: lxd juju-channel: 3.3/stable diff --git a/.github/workflows/promote-charms.yaml b/.github/workflows/promote-charms.yaml index a06101ac..ddac359f 100644 --- a/.github/workflows/promote-charms.yaml +++ b/.github/workflows/promote-charms.yaml @@ -67,7 +67,7 @@ jobs: strategy: matrix: charm-directory: ${{ fromJson(needs.select-charms.outputs.charms) }} - uses: canonical/operator-workflows/.github/workflows/promote_charm.yaml@main + uses: canonical/operator-workflows/.github/workflows/promote_charm.yaml@08c5a65a0bc4696164b4f85a29a9ccbd830d10d8 with: origin-channel: ${{needs.configure-track.outputs.track}}/${{ github.event.inputs.origin-risk }} destination-channel: ${{needs.configure-track.outputs.track}}/${{ github.event.inputs.destination-risk }} diff --git a/.github/workflows/publish-charms.yaml b/.github/workflows/publish-charms.yaml index d9a76b6b..32b47d88 100644 --- a/.github/workflows/publish-charms.yaml +++ b/.github/workflows/publish-charms.yaml @@ -32,7 +32,7 @@ jobs: fi publish-to-edge: needs: [configure-channel] - uses: canonical/operator-workflows/.github/workflows/publish_charm.yaml@main + uses: canonical/operator-workflows/.github/workflows/publish_charm.yaml@08c5a65a0bc4696164b4f85a29a9ccbd830d10d8 strategy: matrix: charm: [ diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 967a0893..7d3f7452 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -48,9 +48,6 @@ def pytest_configure(config): """ config.addinivalue_line("markers", "cos: mark COS integration tests.") config.addinivalue_line("markers", "bundle_file(name): specify a YAML bundle file for a test.") - config.addinivalue_line( - "markers", "ignore_blocked: specify if the bundle deploy should ignore BlockedStatus." - ) def pytest_collection_modifyitems(config, items): @@ -214,7 +211,6 @@ async def deploy_model( ops_test: OpsTest, model_name: str, bundle: Bundle, - raise_on_blocked=True, ): """Add a juju model, deploy apps into it, wait for them to be active. @@ -223,7 +219,6 @@ async def deploy_model( ops_test: Instance of the pytest-operator plugin model_name: name of the model in which to deploy bundle: Bundle object to deploy or redeploy into the model - raise_on_blocked: Raise if any unit in the model is blocked Yields: model object @@ -246,7 +241,6 @@ async def deploy_model( await the_model.wait_for_idle( apps=list(bundle.applications), status="active", - raise_on_blocked=raise_on_blocked, timeout=30 * 60, ) yield the_model @@ -260,11 +254,6 @@ async def kubernetes_cluster(request: pytest.FixtureRequest, ops_test: OpsTest): if bundle_marker: bundle_file = bundle_marker.args[0] - raise_on_blocked = True - ignore_blocked = request.node.get_closest_marker("ignore_blocked") - if ignore_blocked: - raise_on_blocked = False - log.info("Deploying cluster using %s bundle.", bundle_file) model = "main" @@ -278,7 +267,7 @@ async def kubernetes_cluster(request: pytest.FixtureRequest, ops_test: OpsTest): bundle.drop_constraints() for path, charm in zip(charm_files, charms): bundle.switch(charm.app_name, path) - async with deploy_model(request, ops_test, model, bundle, raise_on_blocked) as the_model: + async with deploy_model(request, ops_test, model, bundle) as the_model: yield the_model diff --git a/tests/integration/test_etcd.py b/tests/integration/test_etcd.py index d0e3c5f0..52d8c52c 100644 --- a/tests/integration/test_etcd.py +++ b/tests/integration/test_etcd.py @@ -16,7 +16,6 @@ # bundle with etcd, for all the test within this module. pytestmark = [ pytest.mark.bundle_file("test-bundle-etcd.yaml"), - pytest.mark.ignore_blocked, ] From fa14e45459e876ab5159782852652d56281792a3 Mon Sep 17 00:00:00 2001 From: Adam Dyess Date: Mon, 6 May 2024 16:55:36 -0500 Subject: [PATCH 2/2] Pin snap revision for amd64 (#99) * Pin snap revision for amd64 * Improve snap pinning to not perpetually refresh to the same revision --- charms/worker/k8s/src/snap.py | 10 +++++-- .../k8s/templates/snap_installation.yaml | 3 +- charms/worker/k8s/tests/unit/test_snap.py | 29 +++++++++++++++++-- 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/charms/worker/k8s/src/snap.py b/charms/worker/k8s/src/snap.py index 380213c5..e91f49fd 100644 --- a/charms/worker/k8s/src/snap.py +++ b/charms/worker/k8s/src/snap.py @@ -54,7 +54,7 @@ class SnapStoreArgument(BaseModel): devmode (bool): If it should be installed as with dev mode enabled channel (bool): the channel to install from cohort (str): the key of a cohort that this snap belongs to - revision (int): the revision of the snap to install + revision (str): the revision of the snap to install """ install_type: Literal["store"] = Field("store", alias="install-type", exclude=True) @@ -64,7 +64,7 @@ class SnapStoreArgument(BaseModel): state: Optional[snap_lib.SnapState] = Field(snap_lib.SnapState.Present) channel: Optional[str] = None cohort: Optional[str] = None - revision: Optional[int] = None + revision: Optional[str] = None SnapArgument = Annotated[ @@ -113,8 +113,12 @@ def management(): which = cache[args.name] if isinstance(args, SnapFileArgument) and which.revision != "x1": snap_lib.install_local(**args.dict(exclude_none=True)) + elif isinstance(args, SnapStoreArgument) and args.revision: + if which.revision != args.revision: + log.info("Ensuring %s snap revision=%s", args.name, args.revision) + which.ensure(**args.dict(exclude_none=True)) elif isinstance(args, SnapStoreArgument): - log.info("Ensuring %s snap version", args.name) + log.info("Ensuring %s snap channel=%s", args.name, args.channel) which.ensure(**args.dict(exclude_none=True)) diff --git a/charms/worker/k8s/templates/snap_installation.yaml b/charms/worker/k8s/templates/snap_installation.yaml index 31244c6d..6340b9e7 100644 --- a/charms/worker/k8s/templates/snap_installation.yaml +++ b/charms/worker/k8s/templates/snap_installation.yaml @@ -4,7 +4,8 @@ amd64: - name: k8s install-type: store - channel: edge + channel: 1.30-classic/stable + revision: 301 arm64: - name: k8s install-type: store diff --git a/charms/worker/k8s/tests/unit/test_snap.py b/charms/worker/k8s/tests/unit/test_snap.py index b8e0126a..76b9b503 100644 --- a/charms/worker/k8s/tests/unit/test_snap.py +++ b/charms/worker/k8s/tests/unit/test_snap.py @@ -109,17 +109,40 @@ def test_management_installs_local(cache, install_local, args): @mock.patch("snap._parse_management_arguments") @mock.patch("snap.snap_lib.install_local") @mock.patch("snap.snap_lib.SnapCache") -def test_management_installs_store(cache, install_local, args): +def test_management_installs_store_from_channel(cache, install_local, args): """Test installer uses store installer.""" - cache.return_value.__getitem__.return_value = mock.MagicMock(spec=snap.snap_lib.Snap) + cache.return_value.__getitem__.return_value = mock.MagicMock(autospec=snap.snap_lib.Snap) + cache.return_value["k8s"].revision = None args.return_value = [ snap.SnapStoreArgument(name="k8s", channel="edge"), ] snap.management() + cache.return_value["k8s"].revision = 123 + snap.management() + cache.called_once_with() + install_local.assert_not_called() + assert cache()["k8s"].ensure.call_count == 2 + cache()["k8s"].ensure.assert_called_with(state=snap.snap_lib.SnapState.Present, channel="edge") + + +@mock.patch("snap._parse_management_arguments") +@mock.patch("snap.snap_lib.install_local") +@mock.patch("snap.snap_lib.SnapCache") +def test_management_installs_store_from_revision(cache, install_local, args): + """Test installer uses store installer.""" + cache.return_value.__getitem__.return_value = mock.MagicMock(autospec=snap.snap_lib.Snap) + cache.return_value["k8s"].revision = None + args.return_value = [ + snap.SnapStoreArgument(name="k8s", revision=123), + ] + snap.management() + cache.return_value["k8s"].revision = "123" + snap.management() cache.called_once_with() install_local.assert_not_called() + assert cache()["k8s"].ensure.call_count == 1 cache()["k8s"].ensure.assert_called_once_with( - state=snap.snap_lib.SnapState.Present, channel="edge" + state=snap.snap_lib.SnapState.Present, revision="123" )