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

licensedevice plugin: Implement docker-based Notifier #1000

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

minor-fixes
Copy link
Contributor

@minor-fixes minor-fixes commented Nov 27, 2023

This change ends up being a sort of "rest of the owl" change to produce
something nominally demo-able. It includes:

  • SQL scripts that initialize a DB with dummy data
  • Docker Notifier implementation that listens for containers to be
    started/killed, and looks at their environment vars to deduce which
    licenses it uses
  • Various other bug fixes and improvements that allow the thing to
    actually work, be demoed.

There are still lots of glaring holes to be fixed in future changes
and/or a rewrite to a non-experimental dir.

Tested:

  • ran locally 2x with:

    NOMAD_PLUGIN_MAGIC_COOKIE=e4327c2e01eabfd75a8a67adb114fb34a757d57eee7728d857a8cec6e91a7255 \
    bazel run \
      //experimental/nomad_resource_plugin/licensedevice/cmd/nomad_license_plugin \
      -- \
      'postgresql://<USER>:<URL_ENCODED_PW>@<HOSTNAME>:5432/cj_stage' \
      'license_state' \
      'scott-workstation-2'
    

(the last arg is the local hostname, which is changed between the two
instances

  • on one instance, ran grpc_cli call unix://<SOCKET_FILE> hashicorp.nomad.plugins.device.DevicePlugin.Fingerprint '' to print
    out streaming Fingerprint responses (plugin prints out the socket file
    to use on start)
  • on the other instance, made a Reserve call, saw Fingerprint respond
    on the first instance as unhealthy
  • ran docker run -it -e LICENSEPLUGIN_RESERVED_IDS=example_ax_1 ubuntu /bin/bash to simulate a workload starting; saw DB get updated and
    more Fingerprint responses
  • exited aforementioned container; saw DB get updated and more
    Fingerprint responses

@minor-fixes minor-fixes force-pushed the nomad_plugin_epoll_impl branch 3 times, most recently from b07a85c to 73d52ea Compare November 30, 2023 14:20
@minor-fixes minor-fixes marked this pull request as ready for review November 30, 2023 14:20
@minor-fixes minor-fixes changed the title licensedevice plugin: Implement epoll-based file Notifier licensedevice plugin: Implement docker-based Notifier Nov 30, 2023
This change ends up being a sort of "rest of the owl" change to produce
something nominally demo-able. It includes:

* SQL scripts that initialize a DB with dummy data
* Docker Notifier implementation that listens for containers to be
  started/killed, and looks at their environment vars to deduce which
  licenses it uses
* Various other bug fixes and improvements that allow the thing to
  actually work, be demoed.

There are still lots of glaring holes to be fixed in future changes
and/or a rewrite to a non-experimental dir.

Tested:
* ran locally 2x with:

  ```
  NOMAD_PLUGIN_MAGIC_COOKIE=e4327c2e01eabfd75a8a67adb114fb34a757d57eee7728d857a8cec6e91a7255 \
  bazel run \
    //experimental/nomad_resource_plugin/licensedevice/cmd/nomad_license_plugin \
    -- \
    'postgresql://<USER>:<URL_ENCODED_PW>@<HOSTNAME>:5432/cj_stage' \
    'license_state' \
    'scott-workstation-2'
 ```

 (the last arg is the local hostname, which is changed between the two
  instances
* on one instance, ran `grpc_cli call unix://<SOCKET_FILE>
  hashicorp.nomad.plugins.device.DevicePlugin.Fingerprint ''` to print
  out streaming Fingerprint responses (plugin prints out the socket file
  to use on start)
* on the other instance, made a `Reserve` call, saw Fingerprint respond
  on the first instance as unhealthy
* ran `docker run -it -e LICENSEPLUGIN_RESERVED_IDS=example_ax_1 ubuntu
  /bin/bash` to simulate a workload starting; saw DB get updated and
  more Fingerprint responses
* exited aforementioned container; saw DB get updated and more
  Fingerprint responses
@@ -32,5 +32,17 @@
},
"go.useLanguageServer": true,
"go.lintOnSave": "off",
"go.vetOnSave": "off"
"go.vetOnSave": "off",
"sqltools.connections": [
Copy link
Contributor

Choose a reason for hiding this comment

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

i dont think these were meant to get in this PR? seems like its not a common connection for all the people in the org

for {
select {
case <-ctx.Done():
break loop
Copy link
Contributor

Choose a reason for hiding this comment

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

unless im missing something...

the jump back to loop will cause this case to get called again no? and at this point it would either cause an infinite loop (assuming the Done() channel is closed), or never reach this case again because the Done() channel is drained.

@mhallmark mhallmark removed their assignment Jan 11, 2024
@minor-fixes minor-fixes marked this pull request as draft January 16, 2024 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants