[Curtin-dev] [Merge] ~lamoura/curtin:disable-networking-config into curtin:master

2020-05-12 Thread Lucas Albuquerque Medeiros de Moura
Lucas Albuquerque Medeiros de Moura has proposed merging ~lamoura/curtin:disable-networking-config into curtin:master. Requested reviews: curtin developers (curtin-dev) For more details, see: https://code.launchpad.net/~lamoura/curtin/+git/curtin/+merge/383785 Currently, there is no easy way

Re: [Curtin-dev] [Merge] ~lamoura/curtin:disable-networking-config into curtin:master

2020-05-13 Thread Lucas Albuquerque Medeiros de Moura
raharper I have addressed the changes you commented. I have created the disabled option on the net_meta module and created a VMTest for this command. I have also created two new VMTests for the {"config": "disabled"} possibility as well. For the case of a network disabled config without a

Re: [Curtin-dev] [Merge] ~lamoura/curtin:disable-networking-config into curtin:master

2020-05-15 Thread Lucas Albuquerque Medeiros de Moura
raharper, I have addressed the proposed changes, Just one doubt about the differences in testing between the disabled config with a vesion key and without it Diff comments: > diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py > index 4afe00c..e0498f6 100644 > ---

Re: [Curtin-dev] [Merge] ~lamoura/curtin:disable-networking-config into curtin:master

2020-05-14 Thread Lucas Albuquerque Medeiros de Moura
raharper, I have update the PR with the proposed changes and addressed some concerns related to the version key on the network config Diff comments: > diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py > index 4afe00c..e0498f6 100644 > --- a/curtin/commands/curthooks.py >

Re: [Curtin-dev] [Merge] ~legovini/curtin:pin-flake8 into curtin:master

2020-05-14 Thread Lucas Albuquerque Medeiros de Moura
Review: Approve LGTM -- https://code.launchpad.net/~legovini/curtin/+git/curtin/+merge/383861 Your team curtin developers is requested to review the proposed merge of ~legovini/curtin:pin-flake8 into curtin:master. -- Mailing list: https://launchpad.net/~curtin-dev Post to :

Re: [Curtin-dev] [Merge] ~lamoura/curtin:disable-networking-config into curtin:master

2020-05-14 Thread Lucas Albuquerque Medeiros de Moura
Diff comments: > diff --git a/curtin/commands/apply_net.py b/curtin/commands/apply_net.py > index ddc5056..8e5596b 100644 > --- a/curtin/commands/apply_net.py > +++ b/curtin/commands/apply_net.py > @@ -82,22 +82,23 @@ def apply_net(target, network_state=None, > network_config=None): >

[Curtin-dev] [Merge] ~lamoura/curtin:fix-flake into curtin:master

2020-05-13 Thread Lucas Albuquerque Medeiros de Moura
The proposal to merge ~lamoura/curtin:fix-flake into curtin:master has been updated. Commit message changed to: Fix flake8 E741 warning For more details, see: https://code.launchpad.net/~lamoura/curtin/+git/curtin/+merge/383811 -- Your team curtin developers is requested to review the

Re: [Curtin-dev] [Merge] ~lamoura/curtin:fix-flake into curtin:master

2020-05-13 Thread Lucas Albuquerque Medeiros de Moura
Thanks for pointing that Paride, I have added the commit message. Also, do you think this bug is caused by the commit message ? I am experiencing the same behavior in my other PR and I did not set the commit message there either --

[Curtin-dev] [Merge] ~lamoura/curtin:fix-flake into curtin:master

2020-05-12 Thread Lucas Albuquerque Medeiros de Moura
Lucas Albuquerque Medeiros de Moura has proposed merging ~lamoura/curtin:fix-flake into curtin:master. Requested reviews: curtin developers (curtin-dev) For more details, see: https://code.launchpad.net/~lamoura/curtin/+git/curtin/+merge/383811 Fix flake8 E741 warning -- Your team curtin

Re: [Curtin-dev] [Merge] ~lamoura/curtin:disable-networking-config into curtin:master

2020-05-12 Thread Lucas Albuquerque Medeiros de Moura
Thanks for the review Ryan :) I do recall that you commented about the net_meta options, but I misunderstood what you said for modifying the auto command to not write the network_config and not creating a new option. I will address all of the proposed changes and also thanks for the broader

Re: [Curtin-dev] [Merge] ~raharper/curtin:fix/makefile-drop-nosetest-command into curtin:master

2020-05-07 Thread Lucas Albuquerque Medeiros de Moura
Review: Approve LGTM too -- https://code.launchpad.net/~raharper/curtin/+git/curtin/+merge/383563 Your team curtin developers is requested to review the proposed merge of ~raharper/curtin:fix/makefile-drop-nosetest-command into curtin:master. -- Mailing list: https://launchpad.net/~curtin-dev

Re: [Curtin-dev] [Merge] ~lamoura/curtin:disable-networking-config into curtin:master

2020-05-20 Thread Lucas Albuquerque Medeiros de Moura
The TestNetMetaDisabledModeBaseTestsAbs overrides some of tests that the network disable config should use. For example, this class overrides the test: test_cloudinit_network_passthrough Which is a test that should be performed for the disable cloud config tests. But I can move this override

Re: [Curtin-dev] [Merge] ~lamoura/curtin:disable-networking-config into curtin:master

2020-05-20 Thread Lucas Albuquerque Medeiros de Moura
Wait, I think I missed something for these scenarios: network: {config: disabled} network: {config: disabled, version=1} We should not create the /etc/cloud/cloud.cfg.d/50-curtin-networking.cfg file for them ? I thought that they were valid cloudinit configs and we should propagate them.

Re: [Curtin-dev] [Merge] ~lamoura/curtin:disable-networking-config into curtin:master

2020-05-19 Thread Lucas Albuquerque Medeiros de Moura
Diff comments: > diff --git a/curtin/commands/apply_net.py b/curtin/commands/apply_net.py > index ddc5056..5f1bc23 100644 > --- a/curtin/commands/apply_net.py > +++ b/curtin/commands/apply_net.py > @@ -99,6 +99,11 @@ def apply_net(target, network_state=None, > network_config=None): >

Re: [Curtin-dev] [Merge] ~lamoura/curtin:disable-networking-config into curtin:master

2020-05-19 Thread Lucas Albuquerque Medeiros de Moura
rahaper, I have updated the code with your suggestions. I have also added in the comments the reason why I believe those tests need to be skipped. They are; test_etc_resolvconf test_ip_output With this solution, I have not used the property yet. But you think we can still benefit from it,

Re: [Curtin-dev] [Merge] ~raharper/curtin:fix/partition-verify-flags-msdos-primary-types into curtin:master

2020-05-19 Thread Lucas Albuquerque Medeiros de Moura
I don't know if this is a possible scenario, but is it possible to have a boot type partition that is not marked as bootable ? For example, suppose we have this type of configuration: sfdisk_info_dos = { "label": "dos", "id": "0xb0dbdde1", "device":

Re: [Curtin-dev] [Merge] ~lamoura/curtin:disable-networking-config into curtin:master

2020-05-19 Thread Lucas Albuquerque Medeiros de Moura
raharper, I have fixed the unittest. Also, both of these tests are currently skipped for the disable config scenarios, for both config with and without version key. Diff comments: > diff --git a/tests/unittests/test_commands_net_meta.py > b/tests/unittests/test_commands_net_meta.py > new file

Re: [Curtin-dev] [Merge] ~lamoura/curtin:disable-networking-config into curtin:master

2020-05-20 Thread Lucas Albuquerque Medeiros de Moura
raharper, I have updated the code with the test refactoring. All network disable tests are now in the same file. -- https://code.launchpad.net/~lamoura/curtin/+git/curtin/+merge/383785 Your team curtin developers is subscribed to branch curtin:master. -- Mailing list:

Re: [Curtin-dev] [Merge] ~lamoura/curtin:disable-networking-config into curtin:master

2020-05-21 Thread Lucas Albuquerque Medeiros de Moura
raharper, I have updated the docstrings Diff comments: > diff --git a/tests/vmtests/test_network_disabled.py > b/tests/vmtests/test_network_disabled.py > new file mode 100644 > index 000..cf55030 > --- /dev/null > +++ b/tests/vmtests/test_network_disabled.py > @@ -0,0 +1,73 @@ > +# This

Re: [Curtin-dev] [Merge] ~raharper/curtin:fix/partition-verify-flags-msdos-primary-types into curtin:master

2020-05-21 Thread Lucas Albuquerque Medeiros de Moura
Review: Approve To the best of knowledge, I could not find any issue with the code -- https://code.launchpad.net/~raharper/curtin/+git/curtin/+merge/384133 Your team curtin developers is subscribed to branch curtin:master. -- Mailing list: https://launchpad.net/~curtin-dev Post to :