Detect more images from commonly used crds

[buroa]

Hey Allen,

Not sure if this is the best way forward for this request, but I am trying to collect more images to pre-pull them when I see updates.

Let me know if you want to improve this.

[allenporter]

Seems good to me generally. I'd like to see some more tests for the kind logic.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.33%. Comparing base (5995dd1) to head (3080362).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #833   +/-   ##
=======================================
  Coverage   93.32%   93.33%           
=======================================
  Files          21       21           
  Lines        2412     2415    +3     
=======================================
+ Hits         2251     2254    +3     
  Misses        161      161           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[allenporter]

Seems good to me generally. I'd like to see some more tests for the kind logic.

Looking great. Can you add a test for the KINDS_IMAGE_KEY logic in test_image.py?

[allenporter]

Marking as draft until PR is ready for additional review.

[allenporter]

I'm going to add the test so we can get this in and prepare a release.

[allenporter]

I'm going to add the test so we can get this in and prepare a release.

Oh, i don't have permission to push to your branch. Anyway, here are what the tests look like: https://github.com/allenporter/flux-local/compare/custom-image-tests?expand=1

[buroa]

@allenporter Tests added and ready

[allenporter]

Can you also re-run the snapshot update?

pytest tests/tool/test_get_cluster.py::test_get_cluster --snapshot-update

Not sure if it changed with other stuff I was doing in parallel but its now failing. (We can ignore the diff test)

[buroa]

Can you also re-run the snapshot update?

pytest tests/tool/test_get_cluster.py::test_get_cluster --snapshot-update

Not sure if it changed with other stuff I was doing in parallel but its now failing. (We can ignore the diff test)

❯ pytest tests/tool/test_get_cluster.py::test_get_cluster --snapshot-update
/opt/homebrew/lib/python3.13/site-packages/pytest_asyncio/plugin.py:207: PytestDeprecationWarning: The configuration option "asyncio_default_fixture_loop_scope" is unset.
The event loop scope for asynchronous fixtures will default to the fixture caching scope. Future versions of pytest-asyncio will default the loop scope for asynchronous fixtures to function scope. Set the default fixture loop scope explicitly in order to avoid unexpected behavior in the future. Valid fixture loop scopes are: "function", "class", "module", "package", "session"

  warnings.warn(PytestDeprecationWarning(_DEFAULT_FIXTURE_LOOP_SCOPE_UNSET))
=========================================================================================== test session starts ============================================================================================
platform darwin -- Python 3.13.2, pytest-8.3.4, pluggy-1.5.0
rootdir: /Users/steven/flux-local
configfile: pytest.ini
plugins: syrupy-4.8.2, asyncio-0.25.0, anyio-4.6.2.post1
asyncio: mode=Mode.AUTO, asyncio_default_fixture_loop_scope=None
collected 14 items

tests/tool/test_get_cluster.py ..............                                                                                                                                                        [100%]

----------------------------------------------------------------------------------------- snapshot report summary ------------------------------------------------------------------------------------------
14 snapshots passed.
============================================================================================ 14 passed in 9.93s ============================================================================================

[buroa]

pytest -vv works 100%... so not sure what is going on here. I just gave it a rebase.

[buroa]

@allenporter Yeah, the rebase now shows a ton of errors. I'm at a loss here.

[allenporter]

Yeah i added like 3 or 4 new features yesterday, so not too surprising. Re running with new snapshots after the rebase should help. (Ignore any merge errors in the snapshots, and any snapshot related todiff)

[buroa]

errors.txt

[buroa]

@allenporter I pulled the upstream and tests, so tests are already failing w/o this PR. 7 failed, 175 passed

[allenporter]

@allenporter I pulled the upstream and tests, so tests are already failing w/o this PR. 7 failed, 175 passed

CI ran on the last commit and is succeeding.

The error "flux-local get clusters: error: argument --output/-o: invalid choice: 'json' (choose from diff, yaml)" means something got dropped in the rebase since you're missing that flag I think

[buroa]

@allenporter I pulled the upstream and tests, so tests are already failing w/o this PR. 7 failed, 175 passed

CI ran on the last commit and is succeeding.

The error "flux-local get clusters: error: argument --output/-o: invalid choice: 'json' (choose from diff, yaml)" means something got dropped in the rebase since you're missing that flag I think

I got it all working but I can't get yaml-cluster9-only-images-with-oci to succeed.

flux-local get cluster --path tests/testdata/cluster9/clusters/dev --output yaml --enable-images

[allenporter]

@allenporter I pulled the upstream and tests, so tests are already failing w/o this PR. 7 failed, 175 passed

CI ran on the last commit and is succeeding.
The error "flux-local get clusters: error: argument --output/-o: invalid choice: 'json' (choose from diff, yaml)" means something got dropped in the rebase since you're missing that flag I think

I got it all working but I can't get yaml-cluster9-only-images-with-oci to succeed.

flux-local get cluster --path tests/testdata/cluster9/clusters/dev --output yaml --enable-images

My impression is that these are the new images being added in this PR, and so the new tests I added in another PR also need their snapshots updated with this command:

$ pytest tests/tool/test_get_cluster.py::test_get_cluster[json-cluster-images] --snapshot-update

[buroa]

@allenporter I pulled the upstream and tests, so tests are already failing w/o this PR. 7 failed, 175 passed

CI ran on the last commit and is succeeding.
The error "flux-local get clusters: error: argument --output/-o: invalid choice: 'json' (choose from diff, yaml)" means something got dropped in the rebase since you're missing that flag I think

I got it all working but I can't get yaml-cluster9-only-images-with-oci to succeed.
flux-local get cluster --path tests/testdata/cluster9/clusters/dev --output yaml --enable-images

My impression is that these are the new images being added in this PR, and so the new tests I added in another PR also need their snapshots updated with this command:

$ pytest tests/tool/test_get_cluster.py::test_get_cluster[json-cluster-images] --snapshot-update

The command runs but tests still fail. I'm going to close this PR as it seems undoable.

[allenporter]

My impression is that these are the new images being added in this PR, and so the new tests I added in another PR also need their snapshots updated with this command:

$ pytest tests/tool/test_get_cluster.py::test_get_cluster[json-cluster-images] --snapshot-update

The command runs but tests still fail. I'm going to close this PR as it seems undoable.

The snapshot file is tests/tool/__snapshots__/test_get_cluster.ambr and you have one of them updated in this PR. Need to edit the same file on the test at ~200 above to include the json tests.

[allenporter]

I deleted my previous branch of your PR

$ git branch -D buroa/main

Then checked out this PR again:

$ gh pr checkout 833

Then was able to see the problem on CI -- presumably you see the same error locally.

$ pytest tests/tool/test_get_cluster.py
...
------------------------------------------------------------ snapshot report summary ------------------------------------------------------------
1 snapshot failed. 18 snapshots passed.
============================================================ short test summary info ============================================================
FAILED tests/tool/test_get_cluster.py::test_get_cluster[json-cluster-images] - assert [+ received] == [- snapshot]
========================================================= 1 failed, 18 passed in 27.21s =========================================================

Then was able to update the snapshot locally:

$ pytest tests/tool/test_get_cluster.py  --snapshot-update
================================================== test session starts ==================================================
platform linux -- Python 3.12.3, pytest-8.3.5, pluggy-1.5.0
rootdir: /workspaces/flux-local
configfile: pytest.ini
plugins: asyncio-0.25.3, cov-6.0.0, syrupy-4.8.2
asyncio: mode=Mode.AUTO, asyncio_default_fixture_loop_scope=None
collected 19 items                                                                                                      

tests/tool/test_get_cluster.py ...................                                                                [100%]

------------------------------------------------ snapshot report summary ------------------------------------------------
18 snapshots passed. 1 snapshot updated.
================================================== 19 passed in 27.06s ==================================================

So the tests pass by producing this diff in the snapshot:

# git diff
diff --git a/tests/tool/__snapshots__/test_get_cluster.ambr b/tests/tool/__snapshots__/test_get_cluster.ambr
index 820a215..797022e 100644
--- a/tests/tool/__snapshots__/test_get_cluster.ambr
+++ b/tests/tool/__snapshots__/test_get_cluster.ambr
@@ -141,7 +141,11 @@
                       "oci_repos": [],
                       "helm_releases": [],
                       "config_maps": [],
-                      "secrets": []
+                      "secrets": [],
+                      "images": [
+                          "ceph/ceph:v16.2.6",
+                          "ghcr.io/cloudnative-pg/postgis:17-3.4"
+                      ]
                   },
                   {
                       "name": "infra-controllers",

[allenporter]

I'm going to try to see if I can push to the existing PR or if it needs a new one.

[allenporter]

Can you enable "Allow edits from maintainers" so i can push to your branch or is it too late? https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork#enabling-repository-maintainer-permissions-on-existing-pull-requests

[buroa]

Can you enable "Allow edits from maintainers" so i can push to your branch or is it too late? docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork#enabling-repository-maintainer-permissions-on-existing-pull-requests

It looks like it's already checked

[allenporter]

It looks like it's already checked

Thanks, I must have done the command wrong. Looks like I got it working.

[allenporter]

Thank @buroa appreciate the contribution here! thanks for the patience with all the merge nonsense.

[buroa]

Thank @buroa appreciate the contribution here! thanks for the patience with all the merge nonsense.

Thank you, I could not figure out these tests, I tried lol.