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.

Sign up for GitHub

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

Jump to bottom

feat: add network interface capability #457

Merged
merged 8 commits into from
Jan 20, 2025
Merged

feat: add network interface capability #457

merged 8 commits into from
Jan 20, 2025

Conversation

sandersms
Copy link
Contributor

@sandersms sandersms commented Jan 8, 2025 •
edited
Loading

Add network interface protobuf handling to go dpu cli.

  • Re-factor the cli structure to have the network - evpn and network - intf hierarchy
  • Add list interface to get list of configured network interfaces

Fixed several CI related error reports:

  • Corrected linting issues in files
  • Updated grpc support to remove the deprecated "Dial" function and replace it with "NewClient"
  • Replaced the "go-ping" deprecated repository with the "pro-bing"
  • Updated the golangci-lint configuration to use the new linters and remove deprecated ones
  • Exclude the G115 error in gosec (part of the golangci-lint) to hold off on false integer conversion reports

@sandersms
feat: add network interface capability
cdd8ef2 
Signed-off-by: Mark Sanders <marksanders194@gmail.com>
@sandersms
fix(godpu): update dockerfile for new alpine and golang versions
f59fbf6 
Signed-off-by: Mark Sanders <marksanders194@gmail.com>
github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 8, 2025
View reviewed changes
Dockerfile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: Apache-2.0
# Copyright (c) 2022-2023 Dell Inc, or its subsidiaries.
FROM docker.io/library/golang:1.21.5-alpine3.17 as builder
FROM docker.io/library/golang:1.23.4-alpine3.21 as builder

Check warning

Code scanning / Scorecard

Pinned-Dependencies Medium

score is 0: containerImage not pinned by hash
Click Remediation section below to solve this issue
Dockerfile
@@ -15,7 +15,7 @@
COPY . .
RUN go build -v -o /dpu .

FROM alpine:3.19
FROM alpine:3.21

Check warning

Code scanning / Scorecard

Pinned-Dependencies Medium

score is 0: containerImage not pinned by hash
Remediation tip: pin your Docker image by updating alpine:3.21 to alpine:3.21@sha256:56fa17d2a7e7f168a043a2712e63aed1f8543aeafdcee47c58dcffe38ed51099
Click Remediation section below for further remediation help
@sandersms
fix(godpu): fix docker-compose evpn test command syntax
7b06a2d 
Signed-off-by: Mark Sanders <marksanders194@gmail.com>
@sandersms
fix(godpu): update alpine versions in docker-compose
19a450e 
Signed-off-by: Mark Sanders <marksanders194@gmail.com>
@sandersms
fix(ci): correct opi-test timeout by waiting after start
b654131 
Signed-off-by: Mark Sanders <marksanders194@gmail.com>
@sandersms
feat(interface): add list interface function
441f270 
Signed-off-by: Mark Sanders <marksanders194@gmail.com>
@sandersms sandersms marked this pull request as ready for review January 15, 2025 18:56
@sandersms sandersms requested a review from a team as a code owner January 15, 2025 18:56
@sandersms sandersms linked an issue Jan 15, 2025 that may be closed by this pull request
Add network interface capability #456
Closed
@sandersms
fix(godpu): restore missing test file
607756e 
Signed-off-by: Mark Sanders <marksanders194@gmail.com>
artek-koltun
artek-koltun approved these changes Jan 20, 2025
View reviewed changes
testing/mock-server/server/server.go Outdated
@@ -314,7 +314,7 @@ func FindStub(service, method string, in, out interface{}) error {

if resp.StatusCode != http.StatusOK {
body, _ := io.ReadAll(resp.Body)
return fmt.Errorf(string(body))
return fmt.Errorf("%s", string(body))
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you do not need to format error message, you can use errors.New

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good addition - changes made to address this particular issue.

cmd/network/netintf/netintf.go
if err != nil {
log.Fatalf("could not create gRPC client: %v", err)
}
defer cancel()
Copy link
Collaborator

Choose a reason for hiding this comment

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

would it make sense to create ctx and cancel right here and keep them close to this defer call?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will look at this one to determine if it needs to move or change in future addition.

@sandersms
fix(godpu): change call for error messages with no format required
0e4ba2c 
Signed-off-by: Mark Sanders <marksanders194@gmail.com>
@sandersms sandersms merged commit 2bc0419 into opiproject:main Jan 20, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@artek-koltun artek-koltun artek-koltun approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add network interface capability
2 participants
@sandersms @artek-koltun