-
Notifications
You must be signed in to change notification settings - Fork 363
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
Ensure that "antctl version" always outputs the client version #1876
Ensure that "antctl version" always outputs the client version #1876
Conversation
Update antctl command framework to support outputting information to the user even when a client request to the server (Agent or Controller) fails. We leverage this framework to ensure that "antctl version" always outputs the client version. To test this functionality, we introduce an interface for the "client" part of antctl, which is in charge of sending a request to the appropriate server. We mock this interface so it can be used in unit tests. Although in the Antrea codebase, we typically place mocks in a sub-package called "testing", it is not possible here, due to the very flat nature of the antctl package, and to the dependencies between objects. For example, "commandDefinition" depends on "client" and "client" depends on "commandDefinition". Changing this structure would require a large code refactor, so instead we adapt the code generation framework to exceptionally tolerate placing mocks directly in the package. Fixes antrea-io#1872
c57b6fc
to
a77a8ed
Compare
Codecov Report
@@ Coverage Diff @@
## main #1876 +/- ##
=======================================
Coverage ? 43.19%
=======================================
Files ? 200
Lines ? 17244
Branches ? 0
=======================================
Hits ? 7449
Misses ? 8777
Partials ? 1018
Flags with carried forward coverage won't be shown. Click here to find out more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you paste the example output for me to understand?
New anctl behavior:
And kubectl for comparison:
Both exit with code 1 in the error / incomplete case. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pasting the outputs. LGTM.
) | ||
|
||
func RequestErrorFallback() (io.Reader, error) { | ||
return strings.NewReader("{}"), nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that the Transform func will still be called for the fallback response?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
correct, I considered several approaches but that's the one I went for: generate a "fake" server response and let it go through the transform functions as usual. An alternative was to modify the transforms to handle nil
responses, but that seemed more error-prone, more invasive and less flexible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/test-all |
Update antctl command framework to support outputting information to the
user even when a client request to the server (Agent or Controller)
fails. We leverage this framework to ensure that "antctl version" always
outputs the client version.
To test this functionality, we introduce an interface for the "client"
part of antctl, which is in charge of sending a request to the
appropriate server. We mock this interface so it can be used in unit
tests. Although in the Antrea codebase, we typically place mocks in a
sub-package called "testing", it is not possible here, due to the very
flat nature of the antctl package, and to the dependencies between
objects. For example, "commandDefinition" depends on "client" and
"client" depends on "commandDefinition". Changing this structure would
require a large code refactor, so instead we adapt the code generation
framework to exceptionally tolerate placing mocks directly in the
package.
Fixes #1872