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/impish SRU #1284

Merged
merged 135 commits into from Feb 22, 2022
Merged

Ubuntu/impish SRU #1284

merged 135 commits into from Feb 22, 2022

Conversation

holmanb
Copy link
Member

@holmanb holmanb commented Feb 21, 2022

Proposed Commit Message

summary: no more than 70 characters

A description of what the change being made is and why it is being
made, if the summary line is insufficient.  The blank line above is
required. This should be wrapped at 72 characters, but otherwise has
no particular length requirements.

If you need to write multiple paragraphs, feel free.

LP: #NNNNNNN (replace with the appropriate bug reference or remove
this line entirely if there is no associated bug)

Additional Context

Test Steps

Checklist:

  • My code follows the process laid out in the documentation
  • I have updated or added any unit tests accordingly
  • I have updated or added any documentation accordingly

dermotbradley and others added 30 commits November 8, 2021 09:43
Whenever "apk upgrade" is triggered also use the "--available" and
"--update-cache" options to ensure that an up-to-date packages list
is used.
Chef tests attempt to reach out to test URLs, which will get blocked by
our on our openstack installs.
Currently any attempt to run an apt command while another process holds
an apt lock will fail. We should instead wait to acquire the apt lock.

LP: #1944611
This patch address an issue where the use of the "set-name"
directive caused the networkd renderer to fail.

LP: #1949407
Integration test runs get unique log directories at
/tmp/cloud_init_test_logs/$DATE_TIME. Make
/tmp/cloud_init_test_logs/last always point to the most recent
integration test directory.
…#1106)

On Bionic and Xenial, pycloudlib sets user.vendor-data config in lxd
to ensure that lxd-agent is setup on those images.

Adapt the lxd_discovery integration test to assert the appropriate
user.vendor-data config key exists if we are on xenial or bionic.

Also add assertions that /var/lib/cloud/nocloud-net/meta-data still
exists in the images because we want NoCloud to be a viable fallback
datasource if LXD config security.lxddev = false or LXD datasource
discovery encountered an unexpected error.
- Added to list of expected warnings on Oracle when opc user has
  no ssh key
- Added retries to tests that read from syslog as that can sometimes
  take time to reflect in the log
- Updated test_apt.py to remove proxy info into its own test as that
  can cause failures in updating, which will immediately traceback
  out of the module and prevent us from running further class tests
- Updated test_apt.py to use a more updated ppa in the test_keyserver
- Added basic rsyslog test to test_combined.py
- Added basic puppet test as test_puppet.py
Move more tests into test_combined.py and remove the CI mark from module
tests that aren't updated often or don't represent core functionality.
Add growpart integration test and associated unit tests

Additionally, a small runcmd check for a commented line.
canonical#1110)

Also simplify a path and fix a spelling error while in the file
For Debian, the network configure file was named
/etc/network/interfaces.d/50-cloud-init, not the 50-cloud-init.cfg,
related to
https://github.com/canonical/cloud-init/blob/62721ae71057530e41779ff02ce578b7b802a60f/cloudinit/distros/debian.py#L56
the static IP customization on Debian will fail owing to
"source /etc/network/interfaces.d/*.cfg".
This change will fix this issue.

LP: #1950136
…anonical#1108)

LXD now adds cloud-init scoped configuration keys network-config,
user-data and vendor-data. The existing user.user-data,
user.vendor-data, user.network-config and meta-data will be
deprecated in newer LXD.

cloud-init will prefer LXD config keys cloud-init.* keys above
user.* keys even if both are present. Warnings will be emitted
for ignored user.* keys if cloud-init.* overrides are present.

Expectation is that the configuration user.network-config,
user.meta-data, user.user-data and user.vendor-data* keys should
not be present at the same time as the comparable cloud-init.* keys.
Some Vultr Datacenters can experience latency in the connection due
to the location of one of the dependant api's. The timouts need to be
adjusted so this isn't a failure in the future.
…al#1117)

testing: monkeypatch system_info call in unit tests

system_info can make calls that read or write from the filesystem, which
should require special mocking. It is also decorated with 'lru_cache',
which means test authors often don't realize they need to be mocking.
Also, we don't actually want the results from the user's local
machine, so monkeypatching it across all tests should be reasonable.

Additionally, moved some of 'system_info` into a helper function to
reduce the surface area of the monkeypatch, added tests for the new
function (and fixed a bug as a result), and removed related mocks that
should be no longer needed.
This is more consistent with other github repositories, and will prompt
a first-time contributor to read the contributing guidelines before
submitting the pull request.

Additionally, added a summary section to the top, updated some outdated
language, and removed some outdated typing guidance.
"HACKING" was renamed to "CONTRIBUTING", update the PR template
URL accordingly.

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
…ical#1119)

Some references were missed in the removal of the agent command
in PR canonical#799.  This simply removes the remaining references.

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
…canonical#1081)

When cloud-init is configured to show SSH user key fingerprints during
boot two of the same message appears for each user. This appears to be as
the util.multi_log call defaults to send to both console directly and to
stderr (which also goes to console).

This change sends them only to console directly.
Vultr uses 169.254.169.254 for the metadata server. Some distros are
having trouble with this on IPv6 only servers because the route is
not being assigned to the link-local interface by default as it is in
other distros. This change sets that route before attempting to fetch
the metadata avoiding the current issue.
GCE currently fetches metadata after network has come up. There's no
reason we can't fetch at init-local time, so update GCE to fetch at
init-local time to be more performant and consistent with other
datasources.
…SC-622) (canonical#1123)

Allow #cloud-config and cloud-init query to use underscore-delimited
"jinja-safe" key aliases for any instance-data.json keys
containing jinja operator characters.

This provides a means to use Jinja's dot-notation instead of square brackets
and quoting to reference "unsafe" obtain attribute names.

Support for these aliased keys is available to both #cloud-config user-data and
`cloud-init query`.

For example #cloud-config alias access can look like:
  {{ ds.config.user_network_config }}

  - instead of -

  {{ ds.config["user.network-config"] }}
Given that there are additional network management tools that we haven't
yet supported with activators, we should log a warning and continue
without network activation here, especially since this was a no-op for
years.

LP: #1948681
This attempts to standardize unit test file location under test/unittests/
such that any source file located at cloudinit/path/to/file.py may have a
corresponding unit test file at test/unittests/path/to/test_file.py.

Noteworthy Comments:
====================
Four different duplicate test files existed:
test_{gpg,util,cc_mounts,cc_resolv_conf}.py
Each of these duplicate file pairs has been merged together. This is a
break in git history for these files.

The test suite appears to have a dependency on test order. Changing test
order causes some tests to fail. This should be rectified, but for now
some tests have been modified in
tests/unittests/config/test_set_passwords.py.

A helper class name starts with "Test" which causes pytest to try
executing it as a test case, which then throws warnings "due to Class
having __init__()".  Silence by changing the name of the class.

# helpers.py is imported in many test files, import paths change
cloudinit/tests/helpers.py -> tests/unittests/helpers.py

# Move directories:
cloudinit/distros/tests -> tests/unittests/distros
cloudinit/cmd/devel/tests -> tests/unittests/cmd/devel
cloudinit/cmd/tests -> tests/unittests/cmd/
cloudinit/sources/helpers/tests -> tests/unittests/sources/helpers
cloudinit/sources/tests -> tests/unittests/sources
cloudinit/net/tests -> tests/unittests/net
cloudinit/config/tests -> tests/unittests/config
cloudinit/analyze/tests/ -> tests/unittests/analyze/

# Standardize tests already in tests/unittests/
test_datasource -> sources
test_distros -> distros
test_vmware -> sources/vmware
test_handler -> config        # this contains cloudconfig module tests
test_runs -> runs
…#1124)

If we set a dhcp server side like this:
$ cat /var/tmp/cloud-init/cloud-init-dhcp-f0rie5tm/dhcp.leases
lease {
...
option classless-static-routes 31.169.254.169.254 0.0.0.0,31.169.254.169.254
    10.112.143.127,22.10.112.140 0.0.0.0,0 10.112.140.1;
...
}
cloud-init fails to configure the routes via 'ip route add' because to there are
two different routes for 169.254.169.254:

$ ip -4 route add 192.168.1.1/32 via 0.0.0.0 dev eth0
$ ip -4 route add 192.168.1.1/32 via 10.112.140.248 dev eth0

But NetworkManager can handle such scenario successfully as it uses "ip route append".
So change cloud-init to also use "ip route append" to fix the issue:

$ ip -4 route append 192.168.1.1/32 via 0.0.0.0 dev eth0
$ ip -4 route append 192.168.1.1/32 via 10.112.140.248 dev eth0

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

RHBZ: #2003231
Fix dead link to "contributing" page in README
Improve schema validation.

This adds strict validation of config module definitions at testing
time, with plumbing included for future runtime validation. This
eliminates a class of bugs resulting from schemas that have definitions
that are incorrect, but get interpreted by jsonschema as
"additionalProperties" that are therefore ignored.

- Add strict meta-schema for jsonschema unit test validation
- Separate schema from module metadata structure
- Improve type annotations for various functions and data types

Cleanup:
- Remove unused jsonschema "required" elements 
- Eliminate manual memoization in schema.py:get_schema(),
        reference module.__doc__ directly
TheRealFalcon and others added 20 commits February 9, 2022 22:25
Split _get_public_ssh_keys_and_source() into
_get_public_keys_from_imds() and _get_public_keys_from_ovf().

Set _get_public_keys_from_imds() to take a parameter of the
IMDS metadata rather than assuming it is already set in
self.metadata.  This will allow us to move negotation into
local phase where self.metadata may not be set yet.  Update this
method to raise KeyError if IMDS metadata is missing/malformed,
and ValueError if SSH key format is not supported.  Update
get_public_ssh_keys() to catch these errors and fall back to the
OVF/Wireserver keys as needed.

To improve clarity, update register_with_azure_and_fetch_data()
to return the list of SSH keys, rather than bundling them into
a dictionary for updating against the metadata dictionary.

There should be no change in behavior with this refactor.

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
…cal#1244)

Once a valid datasource is detected, publish the following artifacts
to expedite cloud-identification without having to invoke cloud-id from
shell scripts or sheling out from python.
    
These files can also be relied on in systemd ConditionPathExists
directives to limit execution of services and units to specific
clouds.
    
/run/cloud-init/cloud-id:
 - A symlink with content that is the canonical cloud-id of the
   datasource detected. This content is the same lower-case value
   as the output of /usr/bin/cloud-id.

/run/cloud-init/cloud-id-<canonical-cloud-id>:
 - A single file which will contain the canonical cloud-id encoded
   in the filename
* Primarily improved grammar for clarity.
* A few Sphinx/RST syntax fixes.
* Set text width to 79 characters per footer documentation
  where needed.
* Changed "yaml" to "YAML" when used in sentences.
Handlers for per-boot/per-instance/per-once multipart MIME

Add handlers for adding scripts to userdata that can be run at various
frequencies. Scripts of type x-shellscript-per-boot,
x-shellscript-per-instance, or x-shellscript-per-once can be added
to a multipart MIME userdata message as part of instance userdata.
These scripts will then be added to the appropriate per-boot,
per-instance, or per-once directory in /var/lib/cloud/scripts/
during processing of userdata.
Fixes the spaces introduced in canonical#1213

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
…anonical#1261)

Ubuntu Jammy output from lsblk --json now contains
'mountpoints': [...] instead of 'mountpoint' for children devs.

Let our integration test handle either case.
Raise runtime errors for unhandled cases which would cause other
exceptions.  Ignore types for a few cases where a non-trivial
refactor would be required to prevent the warning.

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
…nical#1252)

There are inconsistencies for cryptographic libraries across
major distribution releases.

From a bionic host, which doesn't support yescrypt hashing scheme,
attempting run run crypt.crypt locally using a yescrypt hash
from a Jammmy /etc/shadow file will result in failure to produce an
encrypted password. For "unsupported" hash schemes, crypt.crypt
returns None.

To avoid inconsistencies of python cryptographic libs across Linux
releases, perform the password encryption on the system under test.
)

Eliminated the duplicate code and now run the entire configuration
routine against both public and private interfaces.
Also addressed an inconsistency from our metadata api for ipv6
address configuration.
All currently failing modules are excluded from reporting
errors using follow-imports=silent and an exclusion list.

Future work can whittle down this failing list. This change will
start enforcing new modules and those currently passing.

Includes some minor alphabetical reordering in tox.ini.

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
Use PEP 589 syntax for TypeDict annotation.

Also fixes previously broken typing MetaSchema typing implementation.
…l#1257)

Due to race conditions and caching, IMDS may return stale or incomplete
metadata.  Add some validation to detect these scenarios and report
appropriate telemetry.

Introduce normalize_mac_address() to allow for comparison of mac
addresses, replacing that found inline in:
_generate_network_config_from_imds_metadata()

Add validation of final fetch of IMDS metadata.

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
Pre-provisioned instances report ready early in the local phase and
again in the non-local phase, during setup().  Non-PPS only reports
ready during non-local phase.

Update the process to report ready during the local phase for all
cases.  Only attempt to do so if networking is up to prevent stalling
boot. We've already waited at least 20 minutes for DHCP if we're
provisioning, or 5 minutes for DHCP on normal boot requesting updated
network configuration.

- Extend _report_ready() with pubkey_info and raise exception
  on error to consolidate reporting done in _negotiate() and
  _report_ready().

- Remove setup(), moving relevant logic into crawl_metadata().

- Move remaining _negotiate() logic into _cleanup_markers() and
  _determine_wireserver_pubkey_info().

These changes effectively fix two issues that were present:

(1) _negotiated is incorrectly set to True

When failing to report ready.  _negotiate() squashed the exception and
the return value was not checked.  This was probably masked due to the
forced removal of obj.pkl on Ubuntu instances, but would be preferable
once we start persisting it to prevent unnecessary re-negotiation.

(2) provisioning media is not ejected for non-PPS

_negotiate() did not pass iso_dev parameter when reporting ready.  The
host will ensure this operation takes place, but it is preferable to
eject /dev/sr0 from within the guest when we're done with it.

Lastly, this removes any need for lease file parsing as the wireserver
addressed is tracked for ephemeral DHCP.  A follow-up PR will remove
this now-unused logic.

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
Bump the version in cloudinit/version.py to 22.1 and
update ChangeLog.

LP: #1960939
@holmanb holmanb changed the title Ubuntu/impish Ubuntu/impish SRU Feb 21, 2022
Copy link
Collaborator

@blackboxsw blackboxsw left a comment

Choose a reason for hiding this comment

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

Thanks @holmanb . This PR has a few extra commits beyond what we already released to ubuntu/devel(Jammy). For SRUs, Ubuntu policy and our cloud-init SRU exception doesn't let us typically upload "newer" content into stable releases than what was released to the later series unless there are specific bug fixes for the target.

So, we'll need to make sure we only new-upstream-snapshot the same upstream commitish (edit: commitish from upstream/main) that we released to ubuntu/devel

This should suffice

new-upstream-snapshot b3d9acddd352f2c787ca16e132aa741dac00baef

Copy link
Collaborator

@blackboxsw blackboxsw left a comment

Choose a reason for hiding this comment

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

  • only authorship diffs changed and whitespace/formatting fix
  • no new lintain warnings/errors
  • package build success
  • proper versioning and commit representative of what was officially released to ubuntu/devel b3d9acd

@blackboxsw blackboxsw merged commit 47d7512 into canonical:ubuntu/impish Feb 22, 2022
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