← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] ~clinton-fung/launchpad-buildd:restart-apparmor-after-upgrades into launchpad-buildd:master

 


Diff comments:

> diff --git a/lpbuildd/target/run_ci.py b/lpbuildd/target/run_ci.py
> index 54aca19..fde95b7 100644
> --- a/lpbuildd/target/run_ci.py
> +++ b/lpbuildd/target/run_ci.py
> @@ -65,6 +66,16 @@ class RunCIPrepare(
>          self.backend.run(["apt-get", "-y", "install"] + deps)
>          if self.backend.supports_snapd:
>              self.snap_store_set_proxy()
> +        result = subprocess.run(
> +            ["sudo", "service", "apparmor", "restart"],

The code runs as buildd. My initial attempt just ran it without sudo, and it requests interactive password entry, which fails. See https://unix.stackexchange.com/questions/650826/why-is-this-error-interactive-authentication-required-popping-up which helped me resolve this.

We can certainly add more code to check if the container is focal and is running on amd64, but I think that might be unnecessary since the restart is super fast. Are you concerned about performance or about correctness?

> +            capture_output=True,
> +            text=True,
> +            check=False

Good point; I'll fix this; I changed it to False during troubleshooting the sudo thing and this one slipped through.

> +        )
> +        print("Command output:")
> +        print(result.stdout)
> +        print("Command errors (if any):")
> +        print(result.stderr)

Good point; copy/pasted from my POC code.

>          for snap_name, channel in sorted(self.args.channels.items()):
>              if snap_name not in ("lxd", "lpci"):
>                  self.backend.run(


-- 
https://code.launchpad.net/~clinton-fung/launchpad-buildd/+git/launchpad-buildd/+merge/494587
Your team Launchpad code reviewers is requested to review the proposed merge of ~clinton-fung/launchpad-buildd:restart-apparmor-after-upgrades into launchpad-buildd:master.



References