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

Support two types of network. #55

Merged
merged 5 commits into from
Jul 3, 2018
Merged

Support two types of network. #55

merged 5 commits into from
Jul 3, 2018

Conversation

hwchiu
Copy link
Contributor

@hwchiu hwchiu commented Jul 2, 2018

For the network, we should support the cluster/single network.
For cluster network, it should create network for all nodes. Otherwise use the nodename from the parameter.

@hwchiu hwchiu requested review from John-Lin, chenyunchen and sufuf3 July 2, 2018 07:18
NodeName string `bson:"nodeName,omitempty" json:"nodeName,omitempty"`
CreatedAt *time.Time `bson:"createdAt,omitempty" json:"createdAt,omitempty"`
OVS OVSNetwork `bson:"ovs,omitempty" json:"ovs"`
Fake FakeNetwork `bson:"fake,omitempty" json:"fake"` //FakeNetwork, for restful testing.
Copy link

Choose a reason for hiding this comment

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

We don't need a BSON tag for field Fake

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, 3Q

BridgeName: tName,
PhysicalPorts: []entity.PhysicalPort{},
},
Type: "ovs",
Copy link

Choose a reason for hiding this comment

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

We'd better use type definition for Network.Type in case of typos and invalid values.

type NetworkType string
var OVSNetwork = NetworkType("ovs")
...

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's good, I will modify that.


const (
OVSNetworkType NetworkType = "ovs"
FakeNetworkType NetworkType = "fake"
Copy link

Choose a reason for hiding this comment

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

thank you, that's great

@codecov-io
Copy link

Codecov Report

Merging #55 into develop will decrease coverage by 0.51%.
The diff coverage is 79.48%.

Impacted file tree graph

@@            Coverage Diff             @@
##           develop     #55      +/-   ##
==========================================
- Coverage    80.91%   80.4%   -0.52%     
==========================================
  Files           20      20              
  Lines          566     592      +26     
==========================================
+ Hits           458     476      +18     
- Misses          81      85       +4     
- Partials        27      31       +4
Impacted Files Coverage Δ
src/networkprovider/network.go 100% <100%> (ø) ⬆️
src/networkprovider/ovs.go 75.86% <78.37%> (-5.39%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 58af7ad...e047613. Read the comment docs.

@hwchiu hwchiu merged commit 002ce61 into develop Jul 3, 2018
@hwchiu hwchiu deleted the hwchih/VX-147 branch July 3, 2018 05:29
John-Lin pushed a commit that referenced this pull request Jul 25, 2018
Support two types of network.

Former-commit-id: 5d65bdb1f51bd2d67260b67bd81bd302d0c6e210 [formerly 002ce61]
Former-commit-id: ba8e482cafcf8757fd9333597af0fc30f6616bad
John-Lin pushed a commit that referenced this pull request Jul 25, 2018
Support two types of network.

Former-commit-id: 5d65bdb1f51bd2d67260b67bd81bd302d0c6e210 [formerly 5d65bdb1f51bd2d67260b67bd81bd302d0c6e210 [formerly 002ce61]]
Former-commit-id: ba8e482cafcf8757fd9333597af0fc30f6616bad
Former-commit-id: 880ff4b
John-Lin pushed a commit that referenced this pull request Jul 25, 2018
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.

4 participants