pro enable landscape (SC-1508) #2656

orndorffgrant · 2023-07-17T21:08:09Z

TODO:

break up commits
- tests: bool behave userdata
- tests: support arbitrary config var replacement
- tests: machine-name template var
- tests: better errors in output comparison steps
- tests: long stdin step
- system: is service active helper
- cli: pass everything after "--" as extra_args to action
- pro enable landscape
unit tests
change integration test to use staging server
update resource on staging contract server
fix all integration tests
finalize messages/help text

Why is this needed?

This is pro enable landscape as spec-ed with the landscape team.

Test Steps

The new integration tests require some new secrets

tox -e behave-any -- features/landscape.feature

Also try it out manually

./tools/test-in-lxd.sh mantic
pro attach $token
pro enable landscape

Checklist

I have updated or added any unit tests accordingly
I have updated or added any integration tests accordingly
Changes here need to be documented, and this was done in: pro enable landscape docs #2674 to be completed after merging this to get it in for the impending release

Does this PR require extra reviews?

Yes - help text and messaging should be reviewed by landscape pm - done
No

github-actions · 2023-07-17T21:08:24Z

Jira: SC-1508

GitHub Issues: No GitHub issues are fixed by this PR. (No commits have Fixes: #### references)

Launchpad Bugs: No Launchpad bugs are fixed by this PR. (No commits have LP: #### references)

Documentation: Documentation changes addressed in #2674

👍 this comment to confirm that this is correct.

uaclient/cli.py

orndorffgrant · 2023-07-20T01:08:51Z

@lucasmoura @renanrodrigo not all unit tests or integration tests are updated and passing, but this PR is ready for a preliminary implementation/functional review

uaclient/cli.py

uaclient/entitlements/landscape.py

features/landscape.feature

orndorffgrant · 2023-07-21T01:28:20Z

I'm having some timeout issues when testing manually - integration tests are working fine though. It might be my environment somehow - I'll look into it more tomorrow.

lucasmoura · 2023-07-21T11:00:52Z

While testing this manually, I have found out this scenario here:

root@upro-behave-lunar-system-under-test-0721-104909618369:~# pro enable landscape
One moment, checking your subscription first
Updating package lists
Installing landscape-client
Executing `landscape-config`
disabled

The Landscape client must be started on boot to operate correctly.

Start Landscape client on boot? [Y/n]: n
Aborting Landscape configuration
Unexpected error(s) occurred.
For more details, see the log: /var/log/ubuntu-advantage.log
To file a bug run: ubuntu-bug ubuntu-advantage-tools

Which is generating this entry on the logs:

["2023-07-21T10:58:33.135", "ERROR", "root", "wrapper", 2000, "Unhandled exception, please file a bug", {"exc_info": "Traceback (most recent call last):\n  File \"/usr/lib/python3/dist-packages/uaclient/cli.py\", line 1944, in wrapper\n    return func(*args, **kwargs)\n           ^^^^^^^^^^^^^^^^^^^^^\n  File \"/usr/lib/python3/dist-packages/uaclient/cli.py\", line 2070, in main\n    return_value = args.action(args, cfg=cfg, extra_args=extra_args)\n                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/usr/lib/python3/dist-packages/uaclient/cli.py\", line 239, in new_f\n    return f(cmd_args, *args, **kwargs)\n           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/usr/lib/python3/dist-packages/uaclient/cli.py\", line 222, in new_f\n    return f(*args, **kwargs)\n           ^^^^^^^^^^^^^^^^^^\n  File \"/usr/lib/python3/dist-packages/uaclient/cli.py\", line 264, in new_f\n    return f(args, cfg=cfg, **kwargs)\n           ^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/usr/lib/python3/dist-packages/uaclient/cli.py\", line 206, in new_f\n    retval = f(*args, cfg=cfg, **kwargs)\n             ^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/usr/lib/python3/dist-packages/uaclient/cli.py\", line 1302, in action_enable\n    ent_ret, reason = actions.enable_entitlement_by_name(\n                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/usr/lib/python3/dist-packages/uaclient/actions.py\", line 128, in enable_entitlement_by_name\n    return entitlement.enable()\n           ^^^^^^^^^^^^^^^^^^^^\n  File \"/usr/lib/python3/dist-packages/uaclient/entitlements/base.py\", line 474, in enable\n    ret = self._perform_enable(silent=silent)\n          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/usr/lib/python3/dist-packages/uaclient/entitlements/landscape.py\", line 37, in _perform_enable\n    system.subp(cmd, pipe_stdouterr=self.assume_yes)\n  File \"/usr/lib/python3/dist-packages/uaclient/system.py\", line 644, in subp\n    out, err = _subp(\n               ^^^^^^\n  File \"/usr/lib/python3/dist-packages/uaclient/system.py\", line 594, in _subp\n    stdout=out.decode(\"utf-8\"),\n           ^^^^^^^^^^\nAttributeError: 'NoneType' object has no attribute 'decode'"}]
["2023-07-21T10:58:33.196", "DEBUG", "root", "_subp", 598, "Ran cmd: apt-cache policy ubuntu-advantage-tools, rc: 0 stderr: b''", {}]

lucasmoura · 2023-07-21T11:26:20Z

Same error if you provide invalid credentials and try to register the machine:

Enable script execution? [y/N]: N

You may provide an access group for this computer e.g. webservers.

Access group: test

You may provide tags for this computer e.g. server,precise.

Tags: 
Please wait...

Request a new registration for this computer now? [y/N]: y
Invalid account name or registration key.
Unexpected error(s) occurred.
For more details, see the log: /var/log/ubuntu-advantage.log
To file a bug run: ubuntu-bug ubuntu-advantage-tools

orndorffgrant · 2023-07-21T12:39:49Z

Ah yep this is from the change to use subp I missed a spot where we have to conditionally decode stdout/err

orndorffgrant · 2023-07-21T15:56:35Z

@lucasmoura that issue is now fixed but I'm still having what seem to be network problems from landscape-config only when running manually in a lxd container :(

orndorffgrant · 2023-07-21T17:21:49Z

Okay the problem with my manual test is solved. I was setting machine_token_overlay to /root/overlay.json which is root-readable-only. The landscape monitor process calls ua status --format json as non-root, which fails at config read time when trying to read the overlay. That results in a non-json output because the error happens before parsing the --format json argument.

So I recommended that landscape catch json decode errors when parsing our output, but this is not an actual problem with pro enable landscape - just the particular method I was using to test manually.

lucasmoura

I think once we have the unittest and integration tests ready, we can merge this PR

features/landscape.feature

renanrodrigo · 2023-07-21T22:04:42Z

features/landscape.feature

+        When I run `sudo pro disable landscape` with sudo
+
+        # cleanup
+        Then I reject all pending computers on Landscape


I'd specify on the Landscape server here for explicity

I think I'm going to move this to a "after_all" hook or something so that it cleans up even if the test fails

will do this in a followup: #2676

renanrodrigo · 2023-07-21T22:06:58Z

features/steps/output.py

@@ -48,34 +50,55 @@ def then_not_in_conditional_stdout_does_not_match_regexp(
 def then_stream_does_not_match_regexp(context, stream):
    text = process_template_vars(context, context.text)
    content = getattr(context.process, stream).strip()
-    assert_that(content, not_(matches_regexp(text)))
+    if re.compile(text).search(content) is not None:


Just a question on this - what do we win from moving away from hamcrest here? Is there a benefit not to rely on the functions we were using before?

hamcrest gave errors that look like this:

Assertion Failed: Expected: a string matching 'SERVICE +ENTITLED +STATUS +DESCRIPTION cc-eal +yes +n/a +Common Criteria EAL2 Provisioning Packages esm-apps +yes +enabled +Expanded Security Maintenance for Applications esm-infra +yes +enabled +Expanded Security Maintenance for Infrastructure fips +yes +n/a +NIST-certified core packages fips-updates +yes +n/a +NIST-certified core packages with priority security updates livepatch +yes +n/a +Canonical Livepatch service realtime-kernel +yes +n/a +Ubuntu kernel with PREEMPT_RT patches integrated ros +yes +n/a +Security Updates for the Robot Operating System ros-updates +yes +n/a +All Updates for the Robot Operating System usg +yes +disabled +Security compliance and audit tools Enable services with: pro enable <service>' but: was 'SERVICE ENTITLED STATUS DESCRIPTION\ncc-eal yes n/a Common Criteria EAL2 Provisioning Packages\nesm-apps yes enabled Expanded Security Maintenance for Applications\nesm-infra yes enabled Expanded Security Maintenance for Infrastructure\nfips yes n/a NIST-certified core packages\nfips-updates yes n/a NIST-certified core packages with priority security updates\nlivepatch yes n/a Canonical Livepatch service\nrealtime-kernel yes n/a Ubuntu kernel with PREEMPT_RT patches integrated\n├ generic yes n/a Generic version of the RT kernel (default)\n└ intel-iotg yes n/a RT kernel optimized for Intel IOTG platform\nros yes n/a Security Updates for the Robot Operating System\nros-updates yes n/a All Updates for the Robot Operating System\nusg yes disabled Security compliance and audit tools\n\nEnable services with: pro enable <service>\n\n Account: UA Client Test\n Subscription: server-team-ua-client-ci-uaa\n Valid until: Fri Dec 31 00:00:00 9999 UTC\nTechnical support level: essential'

The problem I was facing is that it is hard to read that really long line at the bottom even when it's wrapped.

The new assertions here will give errors that look like this:

Assertion Failed: Expected to match regexp: SERVICE +ENTITLED +STATUS +DESCRIPTION cc-eal +yes +n/a +Common Criteria EAL2 Provisioning Packages esm-apps +yes +enabled +Expanded Security Maintenance for Applications esm-infra +yes +enabled +Expanded Security Maintenance for Infrastructure fips +yes +n/a +NIST-certified core packages fips-updates +yes +n/a +NIST-certified core packages with priority security updates livepatch +yes +n/a +Canonical Livepatch service realtime-kernel +yes +n/a +Ubuntu kernel with PREEMPT_RT patches integrated ros +yes +n/a +Security Updates for the Robot Operating System ros-updates +yes +n/a +All Updates for the Robot Operating System usg +yes +disabled +Security compliance and audit tools Enable services with: pro enable <service> But got: SERVICE ENTITLED STATUS DESCRIPTION cc-eal yes n/a Common Criteria EAL2 Provisioning Packages esm-apps yes enabled Expanded Security Maintenance for Applications esm-infra yes enabled Expanded Security Maintenance for Infrastructure fips yes n/a NIST-certified core packages fips-updates yes n/a NIST-certified core packages with priority security updates livepatch yes n/a Canonical Livepatch service realtime-kernel yes n/a Ubuntu kernel with PREEMPT_RT patches integrated ├ generic yes n/a Generic version of the RT kernel (default) └ intel-iotg yes n/a RT kernel optimized for Intel IOTG platform ros yes n/a Security Updates for the Robot Operating System ros-updates yes n/a All Updates for the Robot Operating System usg yes disabled Security compliance and audit tools Enable services with: pro enable <service>

Which I find a lot easier to spot the difference in

tools/ua.bash

uaclient/entitlements/__init__.py

uaclient/entitlements/landscape.py

The shown argument should be True when the result will be displayed in the test output. When shown is True, secret config values are not replaced.

I didn't notice we already had an implementation for this and wrote a new one. Then I realized we already had one and decided to combine the new and old implementation in a small refactor.

orndorffgrant · 2023-07-31T16:55:10Z

CI fails are unrelated

orndorffgrant changed the title ~~pro enable landscape~~ wip: pro enable landscape Jul 17, 2023

orndorffgrant added the next label Jul 18, 2023

orndorffgrant changed the title ~~wip: pro enable landscape~~ wip: pro enable landscape (SC-1508) Jul 19, 2023

orndorffgrant force-pushed the pro-enable-landscape branch 2 times, most recently from a0b6004 to 47ef7d0 Compare July 19, 2023 20:55

orndorffgrant commented Jul 19, 2023

View reviewed changes

uaclient/cli.py Outdated Show resolved Hide resolved

orndorffgrant force-pushed the pro-enable-landscape branch 2 times, most recently from 2ce3c52 to 4f70bb1 Compare July 20, 2023 01:06

lucasmoura reviewed Jul 20, 2023

View reviewed changes

orndorffgrant force-pushed the pro-enable-landscape branch from b549260 to 2637a4b Compare July 21, 2023 14:54

orndorffgrant force-pushed the pro-enable-landscape branch from 2637a4b to 35a3a3c Compare July 21, 2023 17:59

lucasmoura approved these changes Jul 21, 2023

View reviewed changes

renanrodrigo reviewed Jul 21, 2023

View reviewed changes

orndorffgrant force-pushed the pro-enable-landscape branch 6 times, most recently from 305ee43 to 2cc2764 Compare July 27, 2023 18:33

orndorffgrant mentioned this pull request Jul 27, 2023

pro enable landscape docs #2674

Merged

orndorffgrant force-pushed the pro-enable-landscape branch 2 times, most recently from c30a64c to e8cec25 Compare July 28, 2023 00:49

renanrodrigo approved these changes Jul 31, 2023

View reviewed changes

orndorffgrant marked this pull request as ready for review July 31, 2023 12:52

orndorffgrant changed the title ~~wip: pro enable landscape (SC-1508)~~ pro enable landscape (SC-1508) Jul 31, 2023

orndorffgrant added 8 commits July 31, 2023 09:40

tests: allow -D args for boolean test config values

7cde206

tests: behave template var for any config value

d787529

The shown argument should be True when the result will be displayed in the test output. When shown is True, secret config values are not replaced.

tests: better assertion errors for failed output comparison steps

e967d21

tests: step for running command with multiline stdin

16f20ca

refactor: simplify helper for querying if systemd unit is active

b693da0

I didn't notice we already had an implementation for this and wrote a new one. Then I realized we already had one and decided to combine the new and old implementation in a small refactor.

cli: pass everything after -- as extra_args to action

7d7a510

contract: support requiredPackages directive

1c24b60

system: support not piping subprocess stdouterr

235d2e7

orndorffgrant force-pushed the pro-enable-landscape branch from e8cec25 to 24e78e4 Compare July 31, 2023 14:14

landscape: support enabling and disabling landscape-client

16681ba

orndorffgrant force-pushed the pro-enable-landscape branch from 24e78e4 to 16681ba Compare July 31, 2023 15:16

orndorffgrant merged commit f793ea4 into main Jul 31, 2023
21 of 23 checks passed

orndorffgrant deleted the pro-enable-landscape branch July 31, 2023 16:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pro enable landscape (SC-1508) #2656

pro enable landscape (SC-1508) #2656

orndorffgrant commented Jul 17, 2023 •

edited

Loading

github-actions bot commented Jul 17, 2023 •

edited

Loading

orndorffgrant commented Jul 20, 2023

orndorffgrant commented Jul 21, 2023

lucasmoura commented Jul 21, 2023

lucasmoura commented Jul 21, 2023

orndorffgrant commented Jul 21, 2023

orndorffgrant commented Jul 21, 2023 •

edited

Loading

orndorffgrant commented Jul 21, 2023 •

edited

Loading

lucasmoura left a comment

renanrodrigo Jul 21, 2023

orndorffgrant Jul 26, 2023

orndorffgrant Jul 31, 2023

renanrodrigo Jul 21, 2023

orndorffgrant Jul 26, 2023

renanrodrigo Jul 31, 2023

orndorffgrant commented Jul 31, 2023

pro enable landscape (SC-1508) #2656

pro enable landscape (SC-1508) #2656

Conversation

orndorffgrant commented Jul 17, 2023 • edited Loading

Why is this needed?

Test Steps

Checklist

Does this PR require extra reviews?

github-actions bot commented Jul 17, 2023 • edited Loading

orndorffgrant commented Jul 20, 2023

orndorffgrant commented Jul 21, 2023

lucasmoura commented Jul 21, 2023

lucasmoura commented Jul 21, 2023

orndorffgrant commented Jul 21, 2023

orndorffgrant commented Jul 21, 2023 • edited Loading

orndorffgrant commented Jul 21, 2023 • edited Loading

lucasmoura left a comment

Choose a reason for hiding this comment

renanrodrigo Jul 21, 2023

Choose a reason for hiding this comment

orndorffgrant Jul 26, 2023

Choose a reason for hiding this comment

orndorffgrant Jul 31, 2023

Choose a reason for hiding this comment

renanrodrigo Jul 21, 2023

Choose a reason for hiding this comment

orndorffgrant Jul 26, 2023

Choose a reason for hiding this comment

renanrodrigo Jul 31, 2023

Choose a reason for hiding this comment

orndorffgrant commented Jul 31, 2023

orndorffgrant commented Jul 17, 2023 •

edited

Loading

github-actions bot commented Jul 17, 2023 •

edited

Loading

orndorffgrant commented Jul 21, 2023 •

edited

Loading

orndorffgrant commented Jul 21, 2023 •

edited

Loading