Add antctl get fqdncache #6868
Add antctl get fqdncache #6868
Conversation
Dhruv-J
force-pushed
the
antctl-get-fqdncache
branch
3 times, most recently
from
ae05e95 to
6a561ee
Compare
This PR adds functionality for a new antctl command: antctl get fqdncache This command fetches the DNS mapping entries for FQDN policies by reading the cache for DNS entries and outputting FQDN name, associated IP, and expiration time. It can also be used with a --domain (-d) flag that can be specified to filter the result for only a certain FQDN name. Signed-off-by: Dhruv-J <[email protected]>
Dhruv-J
force-pushed
the
antctl-get-fqdncache
branch
2 times, most recently
from
0f68993 to
17487a4
Compare
Signed-off-by: Dhruv-J <[email protected]>
Signed-off-by: Dhruv-J <[email protected]>
Signed-off-by: Dhruv-J <[email protected]>
|
/test-all |
Signed-off-by: Dhruv-J <[email protected]>
Signed-off-by: Dhruv-J <[email protected]>
Signed-off-by: Dhruv-J <[email protected]>
|
/test-conformance |
| "time" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
| "gotest.tools/assert" |
There was a problem hiding this comment.
use github.com/stretchr/testify/assert to avoid extra dependency added in go.mod
hack/update-codegen.sh
Outdated
| @@ -34,6 +34,7 @@ IMAGE_NAME="antrea/codegen:kubernetes-1.31.1-build.1" | |||
| # to the "safe" list in the Git config when starting the container (required | |||
| # because of user mismatch). | |||
| if git_status=$(git status --porcelain --untracked=no 2>/dev/null) && [[ -n "${git_status}" ]]; then | |||
| echo ${git_status} | |||
multicluster/hack/update-codegen.sh
Outdated
| @@ -34,6 +34,7 @@ IMAGE_NAME="antrea/codegen:kubernetes-1.31.1-build.1" | |||
| # to the "safe" list in the Git config when starting the container (required | |||
| # because of user mismatch). | |||
| if git_status=$(git status --porcelain --untracked=no 2>/dev/null) && [[ -n "${git_status}" ]]; then | |||
| echo ${git_status} | |||
pkg/agent/apis/types.go
Outdated
| return false | ||
| } | ||
|
|
||
| // maybe some helper funcs needed here to help parse through ^^^^ |
There was a problem hiding this comment.
Can you explain what this means
There was a problem hiding this comment.
was a dev note, removing now, as functionality has been added
| name: "FQDN cache exists", | ||
| expectedStatus: http.StatusOK, | ||
| expectedResponse: []types.DnsCacheEntry{ | ||
| { |
There was a problem hiding this comment.
Let's also add UT cases for 1. multiple addresses for the same FQDN 2. multiple FQDN <-> address mappings
| for _, dnsCacheEntryExp := range expectedEntryList { | ||
| found := false | ||
| for j, dnsCacheEntryRet := range returnedList { | ||
| if !found && dnsCacheEntryExp.FqdnName == dnsCacheEntryRet.FqdnName { |
There was a problem hiding this comment.
Did you try if assert.ElementsMatch works out of the box for comparing those lists?
| } | ||
| } | ||
|
|
||
| func (r FqdnCacheResponse) SortRows() bool { |
There was a problem hiding this comment.
why is the default option for sorting false?
There was a problem hiding this comment.
the rows of the table won't need to be sorted, as there's no filter or flag for it, so the default is false
| assert.Equal(t, tt.expectedStatus, recorder.Code) | ||
| if tt.expectedStatus == http.StatusOK { | ||
| var received []map[string]interface{} | ||
| err = json.Unmarshal(recorder.Body.Bytes(), &received) |
There was a problem hiding this comment.
any tests to add to check for marshal and unmarshal operation correctly?
There was a problem hiding this comment.
triggering this error manually would be difficult, as the json transformation is not something the test is directly in control of, it is also not tested on other PRs which add antctl functionality
| t.Run(tt.name, func(t *testing.T) { | ||
| reqByte, _ := json.Marshal(tt.fqdnList) | ||
| reqReader := bytes.NewReader(reqByte) | ||
| result, err := Transform(reqReader, false, tt.opts) |
There was a problem hiding this comment.
any test to add specific transform function?
There was a problem hiding this comment.
the transform function is tested as part of response.go, if it fails, then the test will fail as well
| expirationTime time.Time | ||
| } | ||
|
|
||
| func (r FqdnCacheResponse) GetTableHeader() []string { |
There was a problem hiding this comment.
can you check if we can add specific test cases to cover this in code cov
There was a problem hiding this comment.
it seems to be left uncovered conventionally, as it is a simple method that primarily relies on the Transform() function to execute correctly, I don't think unit tests should be added for these funcs
|
there's also kind e2e test noEncap mode also failing |
Signed-off-by: Dhruv-J <[email protected]>
|
/test-conformance |
This PR adds functionality for a new antctl command: antctl get fqdncache This command fetches the DNS mapping entries for FQDN policies by reading the cache for DNS entries and outputting FQDN name, associated IP, and expiration time. It can also be used with a --domain (-d) flag that can be specified to filter the result for only a certain FQDN name.