Skip to content

Feature: support get ipvs info - #14

Open
kwanhur wants to merge 3 commits into
moby:masterfrom
kwanhur:feature-info
Open

Feature: support get ipvs info#14
kwanhur wants to merge 3 commits into
moby:masterfrom
kwanhur:feature-info

Conversation

@kwanhur

Copy link
Copy Markdown
Contributor

fetch ipvs info with API info := i.GetInfo()

fetch version number just call info.Version.String(), example: 1.2.1
fetch connection table size call info.ConnTableSize, example: 4096 .

Signed-off-by: kwanhur <huang_hua2012@163.com>
Comment threadipvs.go Outdated

ver := uint(res.version)
return &Info{
Version: &Version{

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we put the conversion of ver to Version in a separate function so we can add unit tests?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

version as one attribute of ipvsInfo, we should test it as a whole? like test case TestInfo

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@kwanhur I agree with @andrewsykim.
We can test different inputs for the conversion and not rely on the netlink of the test system.

Comment threadnetlink.go
}

// parseInfo given a ipvs netlink response this function will respond with a valid info entry, an error otherwise
func (i *Handle) parseInfo(msg []byte) (*ipvsInfo, error) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this needs to be bound to *Handle?

Signed-off-by: kwanhur <huang_hua2012@163.com>
Signed-off-by: kwanhur <huang_hua2012@163.com>
@kwanhur

Copy link
Copy Markdown
ContributorAuthor

@andrewsykim@cpuguy83 add two api GetVersionGetConnectionTableSize, review them pls :-)

Sign up for freeto 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.

3 participants

@kwanhur@cpuguy83@andrewsykim