On Fri, 2019-12-06 at 18:53 +0000, Daniel P. Berrangé wrote:
> +++ b/guests/lcitool
> + def _get_openvz_repo(self):
> + basedir = os.path.dirname(sys.argv[0])
> + repofile = os.path.join(basedir, "playbooks", "update", "templates", "openvz.repo.j2")
This should be
base = Util.get_base()
repofile = os.path.join(base, ...)
> + def _get_openvz_key(self):
> + basedir = os.path.dirname(sys.argv[0])
> + repofile = os.path.join(basedir, "playbooks", "update", "templates", "openvz.key")
Same here, except you probably want to call it keyfile instead of
repofile.
> @@ -723,6 +735,16 @@ class Application:
> elif os_name == "CentOS" and os_version == "7":
> + repo = self._get_openvz_repo()
> + repocmd = "\\n\\\n".join(repo.split("\n"))
> + key = self._get_openvz_key()
> + keycmd = "\\n\\\n".join(key.split("\n"))
> +
> + sys.stdout.write(
> + "RUN echo -e '%s' > /etc/yum.repos.d/openvz.repo && \\\n" % repocmd +
> + " echo -e '%s' > /etc/pki/rpm-gpg/RPM-GPG-KEY-OpenVZ && \\\n" % keycmd +
> + " rpm --import /etc/pki/rpm-gpg/RPM-GPG-KEY-OpenVZ")
This is different from what's right above and below it for, as far
as I can tell, no good reason.
You can make it nicer and more consistent like
repo = self._get_openvz_repo()
key = self._get_openvz_key()
varmap["repo"] = "\\n\\\n".join(repo.split("\n"))
varmap["key"] = "\\n\\\n".join(key.split("\n"))
sys.stdout.write(textwrap.dedent("""
RUN echo -e '{repo}' > /etc/yum.repos.d/openvz.repo && \\
echo -e '{key}' > /etc/pki/rpm-gpg/RPM-GPG-KEY-OpenVZ && \\
rpm --import /etc/pki/rpm-gpg/RPM-GPG-KEY-OpenVZ
""").format(**varmap))
sys.stdout.write(textwrap.dedent("""
RUN {package_manager} update -y && \\
{package_manager} install -y epel-release && \\
{package_manager} install -y {pkgs} && \\
{package_manager} autoremove -y && \\
{package_manager} clean all -y
""").format(**varmap))
or even merge the two RUN statements to reduce the number of layers
that will end up in the resulting image.
Everything else looks good, so with the above changed
Reviewed-by: Andrea Bolognani <abologna@xxxxxxxxxx>
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
