Merge ~alfonsosanchezbeato/network-manager:test-ci into network-manager:snap-1.10

Proposed by Alfonso Sanchez-Beato
Status: Merged
Approved by: Tony Espy
Approved revision: 44bb1fd711241a032021ea6780d926c7aff51f6e
Merged at revision: 068ecec4f35b4a37eb2c0abb971398fce492cee9
Proposed branch: ~alfonsosanchezbeato/network-manager:test-ci
Merge into: network-manager:snap-1.10
Diff against target: 2793 lines (+2389/-17)
50 files modified
debian/patches/series (+1/-0)
debian/patches/snap-support-ppp.patch (+134/-0)
docs/snap/configure-cellular-connections.md (+124/-0)
docs/snap/configure-wifi-connections.md (+80/-0)
docs/snap/edit-connections.md (+155/-0)
docs/snap/enable-ethernet-support.md (+38/-0)
docs/snap/explore-network-status.md (+52/-0)
docs/snap/faq.md (+18/-0)
docs/snap/index.md (+42/-0)
docs/snap/installation.md (+62/-0)
docs/snap/logging-messages.md (+43/-0)
docs/snap/metadata.yaml (+49/-0)
docs/snap/reference/available-commands.md (+19/-0)
docs/snap/reference/configuration/ethernet_support.md (+47/-0)
docs/snap/reference/dbus-api.md (+9/-0)
docs/snap/reference/snap-configuration/debug.md (+51/-0)
docs/snap/reference/snap-configuration/wifi-powersave.md (+38/-0)
docs/snap/reference/snap-configuration/wowlan.md (+126/-0)
docs/snap/release-notes.md (+35/-0)
docs/snap/report-bug.md (+36/-0)
docs/snap/routing-tables.md (+96/-0)
run-tests.sh (+88/-0)
snap-common/bin/snap-config.sh (+7/-8)
snap-patch/resolvconf.patch (+31/-0)
snap/snapcraft.yaml (+21/-9)
spread.yaml (+85/-0)
tests/full/correct-dns-setup/task.yaml (+47/-0)
tests/full/immutable-netplan-config/task.yaml (+10/-0)
tests/full/ipv6-address-assignment/task.yaml (+27/-0)
tests/full/ipv6-router-advertisements/task.yaml (+70/-0)
tests/full/no-netplan-default-config/task.yaml (+41/-0)
tests/full/system-network-is-active/task.yaml (+9/-0)
tests/full/wol-enabled-by-default/task.yaml (+60/-0)
tests/lib/prepare-all.sh (+63/-0)
tests/lib/prepare.sh (+46/-0)
tests/lib/restore-each.sh (+40/-0)
tests/lib/snap-names.sh (+7/-0)
tests/lib/utilities.sh (+166/-0)
tests/main/aliases/task.yaml (+13/-0)
tests/main/can-exec-iptables/task.yaml (+11/-0)
tests/main/debug-config-option/task.yaml (+27/-0)
tests/main/documentation-builds/task.yaml (+14/-0)
tests/main/ethernet-managed-by-networkmanager/task.yaml (+16/-0)
tests/main/installation/task.yaml (+17/-0)
tests/main/set-hostname/task.yaml (+17/-0)
tests/main/static-ip-configuration/task.yaml (+49/-0)
tests/main/wifi-connect-secured-ap/task.yaml (+14/-0)
tests/main/wifi-powersave-config-option/task.yaml (+31/-0)
tests/main/wifi-wowlan-config-option/task.yaml (+77/-0)
tests/main/wifi-wowlan-enabled-correctly/task.yaml (+30/-0)
Reviewer Review Type Date Requested Status
Tony Espy Approve
System Enablement Bot continuous-integration Approve
Review via email: mp+368284@code.staging.launchpad.net

Commit message

* Add iptables to the snap
* Do not call nmcli while NM is not running
* Add spread tests

Description of the change

* Add iptables to the snap
* Do not call nmcli while NM is not running
* Add spread tests

To post a comment you must log in.
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Tony Espy (awe) wrote :

All looks good, however I have a question about your inclusion of resolvconf in the snap:

snap: add pppd and resolconf scripts to the snap

These files cannot be accessed from the snap anymore in UC18. Note that
the resolvconf scripts will be used only when installed in a UC16
system.

I think what you mean is that "these files" are no longer accessible from the core18 base snap.

Also instead of copying the scripts directly into the snap, a better approach would be to consume the resolvconf debian package via 'stage-packages'.

review: Needs Fixing
Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

@Tony, correct, that is what I was referring to. I have changed the commit message to reflect that, and also added an explanation on why I included the resolvconf files:

<<
    snap: add pppd and resolvconf scripts to the snap

    These files cannot be accessed from the snap anymore as they are not
    included in the core18 base. Note that the resolvconf scripts will be
    used only when installed in a UC16 system.

    The resolvconf files had to be modified to use the right folders, that
    is the reason for including them instead of staging from the debian
    package.
>>

Revision history for this message
Tony Espy (awe) wrote :

> @Tony, correct, that is what I was referring to. I have changed the commit
> message to reflect that, and also added an explanation on why I included the
> resolvconf files:
>
> <<
> snap: add pppd and resolvconf scripts to the snap
>
> These files cannot be accessed from the snap anymore as they are not
> included in the core18 base. Note that the resolvconf scripts will be
> used only when installed in a UC16 system.
>
> The resolvconf files had to be modified to use the right folders, that
> is the reason for including them instead of staging from the debian
> package.
> >>

Sure, but you couldn't you do this by applying patches to the files pulled out of the stage-package? Using your approach we're flying blind wrt to resolvconf CVEs.

Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

@Tony, fair enough. Done.

Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Tony Espy (awe) wrote :

Thanks for the changes, LGTM!

review: Approve
Revision history for this message
Tony Espy (awe) wrote :

One last comment, could you use different feature name branches for your future MPs? Otherwise we end up with duplicate merge commit messages like this:

Merge remote-tracking branch 'abeato/test-ci' into snap-1.10

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches