interfaces/builtin: add "network" interface #587

zyga · Mar 4, 2016

This patch adds the "network" snappy interface. This is somewhat
confusingly associated with the system of interfaces, not the system of
network interfaces.

Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com

niemeyer · Mar 4, 2016

niemeyer reviewed Mar 4, 2016

View changes

s/Snippet// on both, to reduce a bit the verbosity. It's very clear without it already.

niemeyer · Mar 4, 2016

niemeyer reviewed Mar 4, 2016

View changes

"network slots are reserved for the operating system snap"

niemeyer · Mar 4, 2016

LGTM, just a couple of trivial comments.

mvo5 · Mar 7, 2016

Looks good.

elopio · Mar 7, 2016

@zyga this is missing a snap that uses the new interface in https://github.com/ubuntu-core/snappy/tree/master/integration-tests/data/snaps + a test that confirms that it can be build, installed and used. Can you please make a new PR for this?

niemeyer · Mar 7, 2016

@elopio We need to figure someone else to do those integration tests if we want @zyga to get to the end of the requirements for 16.04. Any suggestions or should I look for resources?

elopio · Mar 7, 2016

@niemeyer The plan and agreement we had was that each developer will write their own integration tests, with Federico and I helping. If we have time for developers writing code but not for them to write tests, what we need is to look for more resources writing both code and tests. It makes no sense to get some people writing only production code, and some people writing only tests.

zyga · Mar 7, 2016

@elopio this is not testable yet, once we get to the end of "interfaces work" card all of this code can be only tested with unit tests.

zyga · Mar 7, 2016

@elopio I can do the integration tests (to the extent that I can create snaps that use each interface) but I see this more as a distributed effort (lots of ifaces, lots of various, production snaps) and one that will start after the basics are working.

elopio · Mar 7, 2016

@zyga so, are we still missing code to put this interface in a snap.yaml and build it?

zyga · Mar 7, 2016

@elopio we are missing the state engine actually installing snaps, with the snap manager recording the meta-data in the state so that the interface manager can create interfaces and so that connect/disconnect can operate on that state. It's not all integrated at all yet.

elopio · Mar 7, 2016

@zyga I would prefer for you to put the most basic integration test for each interface that you add, to make sure that it builds, installs and runs. If currently you can only build and install, I would still prefer to have that for each interface as you add it, and then write the last part of the test when all the required bits are ready. We then of course can add tests for more complex scenarios, as we document them.

niemeyer · Mar 7, 2016

@niemeyer The plan and agreement we had was that each developer will write their own integration tests, with Federico and I helping.

@elopio I didn't plan or agree to that.

@zyga I would prefer for you to put the most basic integration test for each interface that you add

@elopio Zyga is working on the foundation of interfaces for people to be able to have something working at all. Can you help us writing the integration tests for those? If you can't, I'll look for resources elsewhere.

Thank you.

elopio · Mar 7, 2016

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 2016-03-07 12:47, Gustavo Niemeyer wrote:

@niemeyer https://github.com/niemeyer The plan and agreement we
had was that each developer will write their own integration
tests, with Federico and I helping.

@elopio https://github.com/elopio I didn't plan or agree to
that.

This has been the plan since I joined the team, and it was discussed
and agreed with zyga and pedronis when they started working on their
features.

@zyga https://github.com/zyga I would prefer for you to put the
most basic integration test for each interface that you add

@elopio https://github.com/elopio Zyga is working on the
foundation of interfaces for people to be able to have something
working at all. Can you help us writing the integration tests for
those? If you can't, I'll look for resources elsewhere.

I can write with zyga the first few tests. I can't write them all. And
as I said, it makes no sense to get new people that will just write
tests. So if you think you can get more resources, bring them, more
hands are always good, but they should be end-to-end developers, not
just late-emergency-testers.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJW3c4GAAoJEIUvYw0No8DgryEQAJpbdrS4JENQsykvOdh8J9eo
r0Q5t0m6VaDh3xdUBKZhb9M+F5xx7xCOgQT7uJlh9XoxbRSdNmO6YCxnEH28N2wF
L+8d65jZBHgm4oXZqUp+WMTWIxUsWKn5KUFpNh+Pn33YrMjVtBUvGtkbZ7VH7Odb
entWR6Eb9AZxwCybjKh46WMe8sK0i60P1oBH9KEhCiGS0pUqf/qff/fw5OpJGMXa
eNqb8sb+QI4QVZgFnSJT2+7aT/mAtubv+my9e3/SYMeFqLOGug6M4S6aFEWb5VwI
9CoIaARvTwQ1o+r3VTv/qJKu0bUmAxF1EWX6oXFF0s4kVCDSwzhZ04b29FTTEXUk
sx1b++o2JoVy83ElvniMSl6DwT6AKpdVAld8z+NfcUtOnPfAt0ZGMLZtA40n9biO
U1ISNAb5tmpWmdA9/LAkytriz41ErnOHPfQ0MJPS3p4b6KFsSDfBQOtq72w+5+XO
eMR7uW9ISFByFHJQETNatE+iuvBk10Xg0Xl8ltRDaaRJXWvGXJ0SPwUxmnJzRyv4
mEHMmPRAnn73G2zJ4lHZTL8eZ/V8mvjtORO2NIypBvwlm2qxPDVznGpkn7IsiSrE
YPedWq3F1mj7iyCMka9odnVM5LyL6+AiKfvO44s6rKNZj01gOsqHinKPMMEfsGsK
BwES+r67LY26aoGAkury
=aNfS
-----END PGP SIGNATURE-----

niemeyer · Mar 7, 2016

This has been the plan since I joined the team, and it was discussed
and agreed with zyga and pedronis when they started working on their
features.

@elopio That's not true (hint: you were not even part of that conversation), but I really don't mind by now.

We have a simple problem, and simple constraints. @zyga is working on the engine, and we're going to have other people doing the actual interfaces. Either you can help us out with integration tests, or we'll find someone else to do so. If you can engage with whoever works on the interfaces (might well come from the security team), that'd be great. If you don't want to help out with that side of things, we'll need to find someone that can.

Both are fine approaches.. I just need to know sooner so we can mobilize the right people.

elopio · Mar 7, 2016

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 2016-03-07 13:05, Gustavo Niemeyer wrote:

This has been the plan since I joined the team, and it was
discussed and agreed with zyga and pedronis when they started
working on their features.

@elopio https://github.com/elopio That's not true (hint: you were
not even part of that conversation), but I really don't mind by
now.

You are clearly talking about a different conversation. Why would I be
untrue?

We have a simple problem, and simple constraints. @zyga
https://github.com/zyga is working on the engine, and we're going
to have other people doing the actual interfaces. Either you can
help us out with integration tests, or we'll find someone else to
do so. If you can engage with whoever works on the interfaces
(might well come from the security team), that'd be great. If you
don't want to help out with that side of things, we'll need to find
someone that can.

zyga wrote this interface, I'm requesting him a test for this
interface. If different people implement the rest of the interfaces, I
will be requesting them to write the tests too. And offering my help
with that, of course, as always. I haven't heard him saying he won't
write the test, or asking for my help with getting started. He just
seemed to be planning to add the test later and more complex than what
I would prefer. I say "seems" because you are not letting us have that
discussion. You came and said no for him, apparently without even
knowing about the amount of time that it will take for him to fulfill
my request.

Both are fine approaches.. I just need to know sooner so we can
mobilize the right people.

I'm sorry, but I'm the one who decides what are good approaches for
testing. That's my job, not yours. And I'm usually pretty nice and
reasonably flexible about it (ask around, because clearly talking to
you is not my strong suit).
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJW3dd7AAoJEIUvYw0No8Dgg7sQAKYqSRMhgcb3RgNxyIXOVqbs
ktGUFiUr30nmH5kSEEjcQ7rU4s5PKzZE+RIE0W9VKkcsw45ciRDSpDNoTeA17baQ
7+mq+ZK840gLbFiwnlDjX3HDRM8GXF9Jz3uYg4Qtsf6886gfvjvRv7WSNeU3Qer/
Htq3e3x4PsfUXl3QmuX3pWI2WOqHM3QeLRXYYSRzYYNeiYRmsbod7f9uIAIXfgAG
pMI51s6I3U9d4DC/QcxLD8jATF2TLnAFQGkm7ilXUFIP7iJ8ZJipKL8GRK9JVM5t
v793j4RZkV8WxupGTwWKCaIoFFilkygEPDZqeAhRuC8DYgN/Kc7g5lTknOwLjH8a
Z/CbgRqK9AC0rDAPgkogoV/6qTpWm5ZrgQZgTGf26BqQOn0kYuIuaRML9Xl/ig2l
HujLO0IYG7xrk4S9WxILIaQgoOVERK6A8nQ1A546UCl26L1NmZ8B1G5Apk2Fo2oG
VqhvaL6c2tHlVezWNcA9qe+pkxtEEw7SCX+kLQaK/b8hT7AbbLXKhAVglxh3BENi
zoy/GCrnWeRNqXGdj1oxq4OLrh0hmv0yIzr4RiAzSP2/vFASOGpU8iL6gJw1Q+0H
QUq6p1b5z7jBojQjBGbAQXMCXPbBacrQBQ/eHs89CHTkHwaP/C0TAWPOkctxxY2P
gSCyXnPvTcESZ8uJRDLJ
=u/Es
-----END PGP SIGNATURE-----

zyga · Mar 7, 2016

@elopio the interface is a straight copy out of ubuntu-core-security (source package).

elopio · Mar 7, 2016

@zyga ok, but I'm still confused. If I do now snappy build on a snap.yaml that has this interface, will it succeed?

niemeyer · Mar 7, 2016

zyga wrote this interface, I'm requesting him a test for this

And I'm saying no, sorry. Please stop bothering @zyga about this, thanks.

I'm sorry, but I'm the one who decides what are good approaches for
testing. That's my job, not yours.

That's both harsh and far from the truth.

Despite your nasty attitude, we'll find some good people to test the interfaces properly. Thanks.

elopio · Mar 7, 2016

ok, I wish you well.

zyga changed the title from interfaces/builtin: add "network" interface. to interfaces/builtin: add "network" interface Mar 4, 2016

mvo5 merged commit 24b086a into snapcore:master Mar 7, 2016
2 checks passed

2 checks passed

Integration tests Success 67 tests run, 0 skipped, 0 failed.
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

zyga deleted the zyga:iface-network branch Mar 7, 2016

+        +		panic(fmt.Sprintf("slot is not of interface %q", iface.Name()))
+        +	}
+        +	if slot.Snap != "ubuntu-core" {
+        +		return fmt.Errorf("slots using the network interface are reserved for ubuntu-core")

snapcore/snapd

interfaces/builtin: add "network" interface #587

zyga commented Mar 4, 2016

zyga changed the title from interfaces/builtin: add "network" interface. to interfaces/builtin: add "network" interface Mar 4, 2016

niemeyer reviewed Mar 4, 2016

View changes

niemeyer reviewed Mar 4, 2016

View changes

niemeyer commented Mar 4, 2016

zyga added some commits Mar 5, 2016

mvo5 commented Mar 7, 2016

mvo5 added a commit that referenced this pull request Mar 7, 2016

mvo5 merged commit `24b086a` into snapcore:master Mar 7, 2016
2 checks passed

2 checks passed

elopio commented Mar 7, 2016

niemeyer commented Mar 7, 2016

elopio commented Mar 7, 2016

zyga commented Mar 7, 2016

zyga commented Mar 7, 2016

elopio commented Mar 7, 2016

zyga commented Mar 7, 2016

zyga deleted the zyga:iface-network branch Mar 7, 2016

elopio commented Mar 7, 2016

niemeyer commented Mar 7, 2016

elopio commented Mar 7, 2016

niemeyer commented Mar 7, 2016

elopio commented Mar 7, 2016

zyga commented Mar 7, 2016

elopio commented Mar 7, 2016

niemeyer commented Mar 7, 2016

elopio commented Mar 7, 2016

snapcore/snapd

Join GitHub today

interfaces/builtin: add "network" interface #587

Conversation

zyga commented Mar 4, 2016

Some checks were not successful

zyga changed the title from interfaces/builtin: add "network" interface. to interfaces/builtin: add "network" interface Mar 4, 2016

niemeyer reviewed Mar 4, 2016 View changes

niemeyer Mar 4, 2016

niemeyer reviewed Mar 4, 2016 View changes

niemeyer Mar 4, 2016

niemeyer commented Mar 4, 2016

zyga added some commits Mar 5, 2016

All checks have passed

mvo5 commented Mar 7, 2016

mvo5 added a commit that referenced this pull request Mar 7, 2016

All checks have passed

Hide details View details mvo5 merged commit 24b086a into snapcore:master Mar 7, 2016 2 checks passed

2 checks passed

elopio commented Mar 7, 2016

niemeyer commented Mar 7, 2016

elopio commented Mar 7, 2016

zyga commented Mar 7, 2016

zyga commented Mar 7, 2016

elopio commented Mar 7, 2016

zyga commented Mar 7, 2016

zyga deleted the zyga:iface-network branch Mar 7, 2016

elopio commented Mar 7, 2016

niemeyer commented Mar 7, 2016

elopio commented Mar 7, 2016

niemeyer commented Mar 7, 2016

elopio commented Mar 7, 2016

zyga commented Mar 7, 2016

elopio commented Mar 7, 2016

niemeyer commented Mar 7, 2016

elopio commented Mar 7, 2016

niemeyer reviewed Mar 4, 2016

View changes

niemeyer reviewed Mar 4, 2016

View changes

mvo5 merged commit `24b086a` into snapcore:master Mar 7, 2016
2 checks passed