Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ubuntu/devel #373

Merged
merged 14 commits into from May 22, 2020
Merged

Ubuntu/devel #373

merged 14 commits into from May 22, 2020

Conversation

blackboxsw
Copy link
Collaborator

Create a new upstream snapshot for upload to groovy
Steps to reproduce:

 cd /tmp
 git clone git@github.com:canonical/cloud-init.git
 git clone git@github.com:canonical/uss-tableflip.git
cd cloud-init
git clone origin/ubuntu/devel -b ubuntu/devel
../uss-tableflip/scripts/new-upstream-snapshot
git diff origin/ubuntu/devel

OddBloke and others added 12 commits May 12, 2020 14:09
canonical#343)

And raise TypeError when subp called with no args, which more accurately mirrors normal behaviour:

>>> from cloudinit.util import subp
>>> subp()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: subp() missing 1 required positional argument: 'args'
Create a schema object for the `apt_configure` module and
validate this schema in the `handle`  function of the module.

There are some considerations regarding this PR:

* The `primary` and `security` keys have the exact same properties. I
  tried to eliminate this redundancy by moving their properties to a
  common place and then just referencing it for both security and
  primary. Similar to what is documented here:
    https://json-schema.org/understanding-json-schema/structuring.html
  under the `Reuse` paragraph.  However, this approach does not work,
  because the `#` pointer goes to the beginning of the file, which is
  a python module instead of a json file, not allowing the pointer to
  find the correct definition. What I did was to create a separate dict
  for the mirror config and reuse it for primary and security, but
  maybe there are better approaches to do that.
* There was no documentation for the config `debconf_selections`. I
  tried to infer what it supposed to do by looking at the code and the
  `debconf-set-selections`  manpage, but my description may not be
  accurate or complete.
* Add a _parse_description function to schema.py to render multi-line
  preformatted content instead of squashing all whitespace

LP: #1858884
This ensures that Travis will not kill our tests if fetching images is
taking a long time.

In implementation terms, this introduces a context manager which will
spin up a multiprocessing.Process in the background and print a dot to
stdout every 10 seconds.  The process is terminated when the context
manager exits.

This also drop the use of travis_wait, which was being used to work
around this issue.
We recently discovered that pylint is failing to report some errors when
invoked across our entire codebase (see
pylint-dev/pylint#3611).  I've run pylint across
every Python file under cloudinit/[0], and this commit fixes the issues
so-discovered.

[0] find cloudinit/ -name "*.py" | xargs -n 1 -t .tox/pylint/bin/python -m pylint
…#368)

We are longer using lxd.readthedocs.io

Signed-off-by: Thomas Parrott thomas.parrott@canonical.com
and slower.
and since we're making it slower, let's cache it, in case boottime gets
called more than once.
Specifically, ensure that given values are either strings, or arrays of strings.
Copy link
Contributor

@lucasmoura lucasmoura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have followed the step @blackboxsw pointed it out and the new upstream release seems okay for me. I tried to verify if the work we have done in this past commits are appearing in the new devel release. But is there a better to verify if everything is fine with the release ?

Also, my changelog was different from the one here. Mine only has the new upstrem release entry. The changelog here is created manually right ?

Copy link
Member

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are we actually reviewing here? Are we supposed to be reviewing the diff or simply verifying that when we follow the procedure we wind up with the same results? I wind up with the same results (minus your last commit as I'd expect)

@blackboxsw
Copy link
Collaborator Author

blackboxsw commented May 20, 2020

What are we actually reviewing here? Are we supposed to be reviewing the diff or simply verifying that when we follow the procedure we wind up with the same results? I wind up with the same results (minus your last commit as I'd expect)

@TheRealFalcon @lucasmoura I should have read this more clearly, ultimately we assume that our new-upstream-snapshot automated tooling is generally correct and we should be able to generate comparable diffs for upload as different developers.

What we are trying to avoid during review is the following:

  • the author put together a new-upstream-snapshot that is stale when compared against the most recent commits in master
  • invalid versioning or target ubuntu release in the debian/changelog
  • incorrectly or poorly formatted debian/changelog entries
  • can the package be built using build-package and sbuild-it

I should have counseled on this in the request for review, I had originally intended this to be an opportunity to make sure we get either of your sbuild environments up and running and using the build-package script so that we can start getting either of you to attempt perform uploads which core developers can then sponsor.

Ultimately, I think we can get you setup and running build-package on your own systems in the subsequent branches that I'll put up for SRU validation. (as we'll have 4 more branches like this to verify)

@blackboxsw
Copy link
Collaborator Author

I have followed the step @blackboxsw pointed it out and the new upstream release seems okay for me. I tried to verify if the work we have done in this past commits are appearing in the new devel release. But is there a better to verify if everything is fine with the release ?

Generally, we should make sure build-package (in uss-tableflip) can build this package from source.
The typical places we have issues with this script:

  • not enough commits represented in the debian/changelog becase author submitting a new-upstream-snapshot that is stale relative the tip of origin/master (so a git fetch origin was forgotten (this may be why your debian/changelog entry differed from mine.
  • the wrong debian/changelog dist target (groovy/focal/xenial) added to the most recent debian changelog entry
  • invalid pkg version suffix chosen for stable releases the (RR,SS.Y of 19.4-33-gbb4131a2-0ubuntu119.10.1)
    • note releasing cloud-init pkg to groovy (ubuntu/devel) won't carry that suffix as it is not a 'stable' ubuntu release

Also, my changelog was different from the one here. Mine only has the new upstrem release entry. The changelog here is created manually right ?
new-upstream-snapshot should generate that changelog manually by tracking the git commits that are present on your local view of origin/master. If your origin/master is stale, you'll only see automated commit summary lines added to debian/changelog for your local 'master' branch.
In this case I'd git checkout master; git pull to ensure you see the latest commits.

@blackboxsw blackboxsw merged commit f4e200b into canonical:ubuntu/devel May 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants