cloud-init icon indicating copy to clipboard operation
cloud-init copied to clipboard

cc_ua: consume ua json api for enable commands

Open aciba90 opened this issue 3 years ago • 1 comments

Proposed Commit Message

cc_ua: consume ua json api for enable commands

Additional Context

https://warthogs.atlassian.net/browse/SC-1253 https://github.com/canonical/cloud-init/pull/1673#discussion_r965352866

Example the UA json api for enable:

$ sudo ua enable --assume-yes --format json asdf livepatch esm-apps esm-apps | jq
{
  "_schema_version": "0.1",
  "errors": [
    {
      "message": "Cannot install Livepatch on a container.",
      "message_code": "livepatch-error-install-on-container",
      "service": "livepatch",
      "type": "service"
    },
    {
      "message": "UA Apps: ESM is already enabled.\nSee: sudo ua status",
      "message_code": "service-already-enabled",
      "service": "esm-apps",
      "type": "service"
    },
    {
      "message": "UA Apps: ESM is already enabled.\nSee: sudo ua status",
      "message_code": "service-already-enabled",
      "service": "esm-apps",
      "type": "service"
    },
    {
      "message": "Cannot enable unknown service 'asdf'.\nTry cc-eal, cis, esm-infra, fips, fips-updates, livepatch.",
      "message_code": "invalid-service-or-failure",
      "service": null,
      "type": "system"
    }
  ],
  "failed_services": [
    "asdf",
    "esm-apps",
    "livepatch"
  ],
  "needs_reboot": false,
  "processed_services": [],
  "result": "failure",
  "warnings": []
}

Test Steps

CLOUD_INIT_UA_TOKEN="<token>" tox -e integration-tests -- tests/integration_tests/modules/test_ubuntu_advantage.py::TestUbuntuAdvantage::test_idempotency

Checklist:

  • [x] My code follows the process laid out in the documentation
  • [x] I have updated or added any unit tests accordingly
  • [ ] I have updated or added any documentation accordingly

aciba90 avatar Oct 05 '22 10:10 aciba90

Thanks for this @aciba90 . I think we minimally need a unittest that'll exercise enable response and exit 1 when a service is already enabled as that should have caught a traceback on "local variable 'enable_cmd_result' referenced before assignment" when subp exits 1 on service-already-enabled.

Sorry I didn't get a full review yet, but wanted to give you something to work with in the morn.

Thanks, @blackboxsw, for the review and comments.

I have reworked the ua enable call to enable all requested services in a single call because:

  1. Efficiency: only one process created
  2. Services could have dependencies between them in the future. Therefore, depending on the order we called ua enable <service> we could fail.

I have added unit-test coverage of all possible error cases and an integration test covering the idempotency case.

aciba90 avatar Oct 06 '22 16:10 aciba90

Thanks, @blackboxsw, for the comments. This PR is ready to be reviewed.

aciba90 avatar Oct 17 '22 11:10 aciba90