Skip to content
This repository has been archived by the owner on Feb 24, 2020. It is now read-only.

networking: Add flags to specify net config and net plugin binary dirs. #2270

Closed
wants to merge 2 commits into from

Conversation

yifan-gu
Copy link
Contributor

Add --net-config for net config dir.
Add --net-plugin for net plugin binary dir.

Also did minor refactoring that groups networking related flags together.

cc @iaguis @alban @jonboulle

Add `--net-config` for net config dir.
Add `--net-plugin` for net plugin binary dir.

Also did minor refactoring that groups networking related flags together.
@yifan-gu
Copy link
Contributor Author

So for the tests, I am trying to copy some plugin binary to a random directory, change its name, and use --net-plugin to point to that directory, to see if rkt can find the binary as expected.

So where should I copy the plugin binary from? ctx.rktBin()/../tools/macvlan ? @krnowak

@yifan-gu yifan-gu changed the title (WIP) networking: Add flags to specify net config and net plugin binary dirs. networking: Add flags to specify net config and net plugin binary dirs. Mar 11, 2016

// Copy and rename the bridge plugin.
// Note that this assumes the 'bridge' plugin locates in build-rkt-${tag}/tools.
if err := os.Link(path.Join(ctx.RktBin(), "..", "..", "..", "tools", "bridge"), path.Join(netPluginDir, "test-bridge")); err != nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@krnowak Could you think of any better way to locate the plugin binary?

@krnowak
Copy link
Collaborator

krnowak commented Mar 14, 2016

@yifan-gu: My idea was to split stage0 and stage1 config as per #2013. So, stage1 config could allow user to specify net plugins paths and CNI config (in a bit different, but consistent format). This, together with passing user config to stage1 should fix your issue too.

I'm not fond of the --net-config flag - my aim was to actually have net plugins config to be in the same format as other config we have (with rktKind and rktVersion fields). I was working on this several weeks ago, but it started to burn me out, so I switched to something else. Of course, this might not be enough for you, if you wanted to have this quickly.

@krnowak
Copy link
Collaborator

krnowak commented Mar 14, 2016

For the record - my work is here - https://github.com/kinvolk/rkt/tree/krnowak/config-split-backward-compatible

@yifan-gu
Copy link
Contributor Author

For the record - my work is here - https://github.com/kinvolk/rkt/tree/krnowak/config-split-backward-compatible

So are we going to get that in for 1.2 or next release? @krnowak

@krnowak
Copy link
Collaborator

krnowak commented Mar 17, 2016

@yifan-gu: For the next one.

@iaguis iaguis modified the milestones: v1.3.0, v1.2.0 Mar 18, 2016
@krnowak
Copy link
Collaborator

krnowak commented Mar 21, 2016

Here is the preliminary PR that implements the feature - #2312

@alban alban modified the milestones: v1.4.0, v1.3.0 Mar 31, 2016
@jonboulle
Copy link
Contributor

Should this be closed in favour of #2312?

@krnowak
Copy link
Collaborator

krnowak commented Apr 13, 2016

Yes.

@krnowak krnowak closed this Apr 13, 2016
@yifan-gu yifan-gu deleted the net_conf branch April 14, 2016 17:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants