Skip to content

feat(compute): select NLB service backends by instance labels - #946

Open
imiric wants to merge 6 commits into
masterfrom
imiric/sc-205653/feat/nlb-label-selector
Open

imiric wants to merge 6 commits into
masterfrom
imiric/sc-205653/feat/nlb-label-selector

Conversation

@imiric

@imiric imiric commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

Description

The API now supports selecting NLB service backends by instance labels instead of relying on Instance Pool membership only.

This is exposed by making --instance-pool optional and adding --label-selector to service add and service update. Requests that would leave a service with no backends are rejected before calling the API, so users get a clear error instead of a 400.

Note

  • A WIP egoscale version is currently vendored because API changes haven't been deployed to prod yet. I'll update this when a stable egoscale version is released.
  • This is ready for review, but should only be merged when CI passes (see the testing note below).

Part of sc-205653

AI-assisted: true
Co-authored-by: qwen3.8-flash-next-iq3_s <noreply@qwen.ai>

Checklist

(For exoscale contributors)

  • Changelog updated (under Unreleased block, and add the Pull Request #number for each bit you add to the CHANGELOG.md)
  • Testing

Testing

E2E tests are expected to fail in CI because API changes are not deployed on prod yet.

I ran the new test manually against preprod, and it passed.

imiric and others added 2 commits October 30, 2026 13:02
Temporary upgrade to github.com/exoscale/egoscale/v3@imiric/sc-205653/feat/nlb-label-selector
The API now supports selecting NLB service backends by instance labels
instead of relying on Instance Pool membership only.

This is exposed by making `--instance-pool` optional and adding
`--label-selector` to `service add` and `service update`. Requests that
would leave a service with no backends are rejected before calling the
API, so users get a clear error instead of a 400.

Part of [sc-205653][1]

[1]: https://app.shortcut.com/exoscale/story/205653/

AI-assisted: true
Co-authored-by: qwen3.8-flash-next-iq3_s <noreply@qwen.ai>
@imiric imiric added the HOLD ✋ Hold, don't merge label Oct 30, 2026
@imiric
imiric requested a review from a team October 30, 2026 13:38

@pierre-emmanuelJ pierre-emmanuelJ left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me. Selector-only services are handled cleanly in show, and the upfront checks on add/update read well. I also checked that --label-selector '[=]' really ends up as an empty map through the flag parsing, not just in the tests. Two small things below. I'll hold off approving until the egoscale release is in and the API is live.


# Setup: create the NLB and an Instance Pool for the pool-backed service
exec exo --output-format json compute load-balancer create ${TEST_RUN_ID}-nlb --zone ${TEST_ZONE}
exec exo --output-format json compute instance-pool create ${TEST_RUN_ID}-pool --zone ${TEST_ZONE}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Until the API is in prod, this scenario fails at line 11, right after creating the NLB and this pool, so the teardown never runs. Each CI run on this PR leaves an NLB and a 1-instance pool (standard.medium) in the CI account. Could you create the pool just before the mix part, and maybe add a skip at the top until the API is deployed?

Repro
$ gh run view 41058585803 --repo exoscale/cli --job 124072267441 --log
> exec exo --output-format json compute instance-pool create ${TEST_RUN_ID}-pool --zone ${TEST_ZONE}
{"id":"d0d8c38a-…","name":"cli-e2e-1793364758-6s4xal-pool",…,"instance_type":"standard.medium",…,"size":1,…}
> exec exo --output-format json compute load-balancer service add ${TEST_RUN_ID}-nlb web --port 8053 --label-selector role=web --zone ${TEST_ZONE}
error: Bad Request: missing keys 'instance-pool'
FAIL: scenarios/with-api/compute/nlb_service_label_selector.txtar:11: unexpected command failure

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You're absolutely right!

Actually... Moving the ipool creation wouldn't address this issue entirely, since a failure at any other point would still leave dangling resources.

So I looked into this a bit, and it turns out that testscript exposes a Defer API which would be a good fit for implementing this type of safe teardown. It does require adding a custom command, but the change is minor. See 121cd1f.

With this command and reordering the removals in the test to happen right after their creation, all resources all cleaned up properly.

Repro
  • Before:
> exo -O json compute instance-pool ls
[]

> exo -O json compute instance ls
[{"id":"204cbdc9-6a97-4730-949e-87ddc21aea60","name":"debian-test","zone":"ch-gva-2","type":"standard.small","ip_address":"92.39.60.199","ipv6_address":"-","state":"stopped"}]

> exo -O json compute nlb ls
[]
  • Prod test:
~/Work/Exoscale/cli/tests/e2e imiric/sc-205653/feat/nlb-label-selector~1
> git rev-parse HEAD
5acadf4c6aa26245d133454beaa61ab293c341fc

> go test -v -tags=api -timeout 30m -run 'TestScriptsAPICompute/nlb_service_label_selector'
=== RUN   TestScriptsAPICompute
    testscript_api_test.go:34: API test run ID: cli-e2e-1793709795-gr8jzz (zone: ch-gva-2)
=== RUN   TestScriptsAPICompute/nlb_service_label_selector
=== PAUSE TestScriptsAPICompute/nlb_service_label_selector
=== CONT  TestScriptsAPICompute/nlb_service_label_selector
    testscript.go:584: WORK=$WORK
        PATH=<redacted>
        GOTRACEBACK=system
        HOME=$WORK
        TMPDIR=$WORK/.tmp
        devnull=/dev/null
        /=/
        :=:
        $=$
        exe=
        XDG_CONFIG_HOME=$WORK/.config
        EXOSCALE_API_KEY=<redacted>
        EXOSCALE_API_SECRET=<redacted>
        EXO_ZONE=ch-gva-2
        EXO_OUTPUT=json
        TEST_RUN_ID=cli-e2e-1793709795-gr8jzz
        TEST_ZONE=ch-gva-2

        # Test: exo compute load-balancer service label selector lifecycle
        # Selector-only service create/show/update, selector removal on a
        # pool-backed service, and client-side validation errors.
        # TEST_ZONE and TEST_RUN_ID are injected by the API test runner.
        # Setup: create the NLB used by all services in this scenario (9.494s)
        > exec exo --output-format json compute load-balancer create ${TEST_RUN_ID}-nlb --zone ${TEST_ZONE}
        [stdout]
        {"id":"ed658e32-95be-4bd5-9dc8-36187c0c385c","name":"cli-e2e-1793709795-gr8jzz-nlb","description":"","creation_date":"2026-11-03 12:43:15 +0000 UTC","zone":"ch-gva-2","ip_address":"185.19.30.88","address_family":"inet4","state":"running","services":[],"labels":null}
        [stderr]
        Creating Network Load Balancer "cli-e2e-1793709795-gr8jzz-nlb"... 9s
        > defer exo compute load-balancer delete --force ${TEST_RUN_ID}-nlb --zone ${TEST_ZONE}
        # Add a service with a label selector and no Instance Pool (27.711s)
        > exec exo --output-format json compute load-balancer service add ${TEST_RUN_ID}-nlb web --port 8053 --label-selector role=web --zone ${TEST_ZONE}
        [stderr]
        error: Bad Request: missing keys 'instance-pool'
        [exit status 1]
        FAIL: scenarios/with-api/compute/nlb_service_label_selector.txtar:11: unexpected command failure
        [stderr]
        Deleting Network Load Balancer "cli-e2e-1793709795-gr8jzz-nlb"... 27s


--- FAIL: TestScriptsAPICompute (0.00s)
    --- FAIL: TestScriptsAPICompute/nlb_service_label_selector (37.21s)
FAIL
exit status 1
FAIL	github.com/exoscale/cli/internal/e2e	37.218s
  • After:
> exo -O json compute instance-pool ls
[]

> exo -O json compute instance ls
[{"id":"204cbdc9-6a97-4730-949e-87ddc21aea60","name":"debian-test","zone":"ch-gva-2","type":"standard.small","ip_address":"92.39.60.199","ipv6_address":"-","state":"stopped"}]

> exo -O json compute nlb ls
[]

We might want to adopt this for other tests as well, but I left that for later.

I still added a temporary skip to the test, to avoid it running unnecessarily for now.

Comment thread CHANGELOG.md Resolved

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

HOLD ✋ Hold, don't merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants