launchpad-reviewers team mailing list archive

[Tobias Koch <tobias.koch@xxxxxxxxxxxxx>]

Review: Resubmit

@cjwatson, ready for re-review.

Diff comments:

> 
> === modified file 'lpbuildd/target/lxd.py'
> --- lpbuildd/target/lxd.py	2018-06-12 23:23:13 +0000
> +++ lpbuildd/target/lxd.py	2018-10-25 07:43:01 +0000
> @@ -277,16 +277,31 @@
>              old_profile.delete()
>  
>          raw_lxc_config = [
> -            ("lxc.aa_profile", "unconfined"),
>              ("lxc.cap.drop", ""),
>              ("lxc.cap.drop", "sys_time sys_module"),
>              ("lxc.cgroup.devices.deny", ""),
>              ("lxc.cgroup.devices.allow", ""),
>              ("lxc.mount.auto", ""),
>              ("lxc.mount.auto", "proc:rw sys:rw"),
> -            ("lxc.network.0.ipv4", ipv4_address),
> -            ("lxc.network.0.ipv4.gateway", self.ipv4_network.ip),
>              ]
> +
> +        lxc_version = self._client.host_info.get("environment", {}).get(
> +                "driver_version")
> +        major, minor = [int(v) for v in lxc_version.split(".")[0:2]]
> +
> +        if major >= 3:
> +            raw_lxc_config.extend([
> +                ("lxc.apparmor.profile", "unconfined"),
> +                ("lxc.net.0.ipv4.address", ipv4_address),
> +                ("lxc.net.0.ipv4.gateway", self.ipv4_network.ip),
> +                ])
> +        else:
> +            raw_lxc_config.extend([
> +                ("lxc.aa_profile", "unconfined"),
> +                ("lxc.network.0.ipv4", ipv4_address),
> +                ("lxc.network.0.ipv4.gateway", self.ipv4_network.ip),
> +                ])
> +

The suggestion was to use a dict and sort later. I suppose sorting the list is going to do the same.

>          # Linux 4.4 on powerpc doesn't support all the seccomp bits that LXD
>          # needs.
>          if self.arch == "powerpc":
> 
> === modified file 'lpbuildd/target/tests/test_lxd.py'
> --- lpbuildd/target/tests/test_lxd.py	2018-05-08 09:37:16 +0000
> +++ lpbuildd/target/tests/test_lxd.py	2018-10-25 07:43:01 +0000
> @@ -181,22 +203,40 @@
>                  "type": "nic",
>                  },
>              }
> +        if driver_version == "3.0":
> +            expected_devices["root"] = {
> +                "path": "/",
> +                "pool": "default",
> +                "type": "disk",
> +                }
>          client.profiles.create.assert_called_once_with(
>              "lpbuildd", expected_config, expected_devices)
>  
>      def test_create_profile_amd64(self):
> -        self.useFixture(MockPatch("pylxd.Client"))
> -        client = pylxd.Client()
> -        client.profiles.get.side_effect = FakeLXDAPIException
> -        LXD("1", "xenial", "amd64").create_profile()
> -        self.assert_correct_profile()
> +        for driver_version in ["2.0", "3.0"]:
> +            with MockPatch("pylxd.Client"):
> +                client = pylxd.Client()
> +                client.profiles.get.side_effect = FakeLXDAPIException
> +                client.host_info = {
> +                    "environment": {"driver_version": driver_version}
> +                    }
> +                LXD("1", "xenial", "amd64").create_profile()
> +                self.assert_correct_profile(
> +                        driver_version=driver_version or "3.0")

The suggestion was to take the fixture out of the loop, but it needs to be reset on each iteration, else checks such as 
client.profiles.get.assert_called_once_with("lpbuildd") will break.

>  
>      def test_create_profile_powerpc(self):
> -        self.useFixture(MockPatch("pylxd.Client"))
> -        client = pylxd.Client()
> -        client.profiles.get.side_effect = FakeLXDAPIException
> -        LXD("1", "xenial", "powerpc").create_profile()
> -        self.assert_correct_profile("lxc.seccomp=\n")
> +        for driver_version in ["2.0", "3.0"]:
> +            with MockPatch("pylxd.Client"):
> +                client = pylxd.Client()
> +                client.profiles.get.side_effect = FakeLXDAPIException
> +                client.host_info = {
> +                    "environment": {"driver_version": driver_version}
> +                    }
> +                LXD("1", "xenial", "powerpc").create_profile()
> +                self.assert_correct_profile(
> +                        extra_raw_lxc_config=[("lxc.seccomp", ""),],
> +                        driver_version=driver_version or "3.0"
> +                        )
>  
>      def test_start(self):
>          fs_fixture = self.useFixture(FakeFilesystem())


-- 
https://code.launchpad.net/~tobijk/launchpad-buildd/launchpad-buildd-bionic/+merge/354331
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~tobijk/launchpad-buildd/launchpad-buildd-bionic into lp:launchpad-buildd.