canonical-ubuntu-qa team mailing list archive

[Tim Andersson <mp+471159@xxxxxxxxxxxxxxxxxx>]

commit 1 all good

one inline comment on commit 2 - log is obsolete next to status.maintenance in some places

We should also look to not include the webcontrol/ and autopkgtest-cloud/ directories in the charms, though we will do this work in ADT-664. This should speed up `charmcraft pack`.

Other than that, LGTM 

Diff comments:

> diff --git a/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py b/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py
> index f2a8ab7..bd6e4e4 100644
> --- a/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py
> +++ b/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py
> @@ -149,6 +161,26 @@ def clone_autopkgtest():
>              log("autopkgtest already cloned", "INFO")
>  
>  
> +@when("apt.installed.git")
> +@when_not("autopkgtest.autopkgtest-cloud_cloned")
> +def clone_autopkgtest_cloud():
> +    status.maintenance("Cloning autopkgtest-cloud")
> +    log("Cloning autopkgtest-cloud", "INFO")

this isn't needed, line above does the same thing - your decision as to whether you want to or not

> +    with UnixUser("ubuntu"):
> +        try:
> +            pygit2.clone_repository(
> +                AUTOPKGTEST_CLOUD_CLONE_LOCATION,
> +                AUTOPKGTEST_CLOUD_GIT_LOCATION,
> +                checkout_branch=AUTOPKGTEST_CLOUD_CLONE_BRANCH,
> +            )
> +            status.maintenance("autopkgtest-cloud cloned")
> +            log("autopkgtest-cloud cloned", "INFO")
> +            set_flag("autopkgtest.autopkgtest-cloud_cloned")
> +        except ValueError:
> +            # exists
> +            log("autopkgtest-cloud already cloned", "INFO")
> +
> +
>  @when_all(
>      "autopkgtest.autopkgtest_cloud_symlinked",
>      "autopkgtest.rabbitmq-configured",
> diff --git a/charms/focal/autopkgtest-web/lib/utils.py b/charms/focal/autopkgtest-web/lib/utils.py
> new file mode 100644
> index 0000000..4f1316c
> --- /dev/null
> +++ b/charms/focal/autopkgtest-web/lib/utils.py
> @@ -0,0 +1,55 @@
> +# pylint: disable=missing-module-docstring, missing-class-docstring, missing-function-docstring
> +import os
> +import pwd
> +
> +from charmhelpers.core.hookenv import log
> +
> +
> +# class UnixUser(object):

leftover comment

> +class UnixUser:
> +    def __init__(self, username):
> +        self.username = username
> +        pwnam = pwd.getpwnam(username)
> +        self.uid = pwnam.pw_uid
> +        self.gid = pwnam.pw_gid
> +
> +    def __enter__(self):
> +        log(
> +            "Raising UID to {uid} and GID to {gid}".format(
> +                uid=self.uid, gid=self.gid
> +            ),
> +            "INFO",
> +        )
> +        os.setresgid(self.gid, self.gid, os.getgid())
> +        os.setresuid(self.uid, self.uid, os.getuid())
> +
> +    def __exit__(self, exc_type, exc_val, exc_tb):
> +        _, _, suid = os.getresuid()
> +        _, _, sgid = os.getresgid()
> +        log(
> +            "Restoring UID to {suid} and GID to {sgid}".format(
> +                suid=suid, sgid=sgid
> +            ),
> +            "INFO",
> +        )
> +        os.setresuid(suid, suid, suid)
> +        os.setresgid(sgid, sgid, sgid)
> +
> +
> +def pull(repository, branch):
> +    """
> +    This will do a sort of git fetch origin && git reset --hard origin/master
> +    This raises pygit2.GitError whenever the tree is not clean, preventing to loose cowboys.
> +    """
> +    origin = [
> +        remote for remote in repository.remotes if remote.name == "origin"
> +    ][0]
> +    origin.fetch()
> +    remote_master_id = repository.lookup_reference(
> +        f"refs/remotes/origin/{branch}"
> +    ).target
> +    repository.checkout_tree(repository.get(remote_master_id))
> +    master_ref = repository.lookup_reference(f"refs/heads/{branch}")
> +    master_ref.set_target(remote_master_id)
> +    repository.head.set_target(remote_master_id)
> +    log(f"Updated repository {repository} to {remote_master_id}", "INFO")


-- 
https://code.launchpad.net/~hyask/autopkgtest-cloud/+git/autopkgtest-cloud/+merge/471159
Your team Canonical's Ubuntu QA is requested to review the proposed merge of ~hyask/autopkgtest-cloud:skia/git_clone_in_charms into autopkgtest-cloud:master.