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.

Sign up for GitHub

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

Jump to bottom

Avoid race between operation and events listener #12885

Closed
wants to merge 7 commits into from
Closed

Avoid race between operation and events listener #12885

wants to merge 7 commits into from

Commits on Mar 18, 2024

  1. client/lxd: Add supportsAuthentication field to ProtocolLXD 

    This field can be used to determine whether the client is capable of
making potentially trusted connections to its target server.

This will be useful when determining whether the client should
automatically spawn an events websocket listener for operations and
other events which require authentication with the
`/1.0/events` endpoint.

Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
    @masnax
    masnax committed Mar 18, 2024
    Configuration menu
    Copy the full SHA
    f26353c View commit details
    Browse the repository at this point in the history

  2. client/lxd/events: Only use events API if client supports auth 

    The /1.0/events API only allows authenticated connections.

When making a request that spawns an operation, we try to
automatically connect to the events API, but the client may
not necessarily support authentication if it is using a
temporary key to establish trust with a particular endpoint.

This leads to an obscure Forbidden error even though the actual
request we made was trusted. Instead, this ensures we don't try to
incorrectly connect to the events API if the underlying ProtocolLXD
does not support authentication.

Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
    @masnax
    masnax committed Mar 18, 2024
    Configuration menu
    Copy the full SHA
    55de107 View commit details
    Browse the repository at this point in the history

  3. client: Create events listeners before making the op request 

    To ensure we have begun listening for events before the operation has
completed, we should explicitly create the events listener for the
operation before actually initiating the request that creates and starts
the operation.

Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
    @masnax
    masnax committed Mar 18, 2024
    Configuration menu
    Copy the full SHA
    1085085 View commit details
    Browse the repository at this point in the history

  4. client/operations: Don't create events listener after starting an ope… 

    …ration

If we implicitly create the events listener after the operation has
fired, then there's a chance the operation will have completed and
emitted the corresponding event before we even begin listening.

Waiting for an operation can thus potentially block indefinitely,
if we first encounter the operation in a running state, but then
it completes before we set up the listener.

Instead, we shouldn't try to create an events listener automatically
when setting up the wait process as we have no way of determining the
current state of an operation yet.

Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
    @masnax
    masnax committed Mar 18, 2024
    Configuration menu
    Copy the full SHA
    72bd241 View commit details
    Browse the repository at this point in the history

  5. client/operations: Remove skipListener field 

    As the `setupListener` method will no longer be responsible for spawning
a listener, we can simply check if the operation has a non-nil events
listener to determine whether we should use /1.0/operations/wait or
/1.0/events to track the operation.

Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
    @masnax
    masnax committed Mar 18, 2024
    Configuration menu
    Copy the full SHA
    5017a40 View commit details
    Browse the repository at this point in the history

  6. client/lxd: Expose SupportsAuthentication field 

    Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
    @masnax
    masnax committed Mar 18, 2024
    Configuration menu
    Copy the full SHA
    861f757 View commit details
    Browse the repository at this point in the history

  7. lxc: Ensure the client supports event handler authentication 

    In many cases, we call `AddHandler` directly after creating an
operation, expecting it to support connection to an events websocket.

This won't be the case if the client is unable to authenticate with that
endpoint.

Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
    @masnax
    masnax committed Mar 18, 2024
    Configuration menu
    Copy the full SHA
    c3a3f44 View commit details
    Browse the repository at this point in the history