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

cni-plugin: Clean up code #26505

Merged
merged 4 commits into from Jun 27, 2023
Merged

cni-plugin: Clean up code #26505

merged 4 commits into from Jun 27, 2023

Conversation

gandro
Copy link
Member

@gandro gandro commented Jun 27, 2023

This PR cleans up the CNI code in preparation for changes coming in subsequent PRs.

  • Review per commit
  • This PR contains no functional changes

This commit introduces more idiomatic error handling to `cmdAdd` and
`cmdDel` in the CNI plugin code. In Golang, if one has a named return
argument such as `func f() (err error)` and one refers to `err` in a
`defer` statement, then it is not necessary to manually assign errors to
`err`. The statement `return foo` will automatically assign `err = foo`
and thus make the value `foo` visible in any defer statement.

This commit contains no functional changes.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
This commit removes a variable which can be inlined, as it is only used
once.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
This commit removes the C89 style "variable declaration block" in the
CNI plugin and moves all variables declaration closer to their actual
initalization point.

This commit contains no functional changes.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
It is not used anywhere. This contains no functional changes.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
@gandro gandro added area/cni Impacts the Container Networking Interface between Cilium and the orchestrator. release-note/misc This PR makes changes that have no direct user impact. labels Jun 27, 2023
@gandro gandro requested a review from a team as a code owner June 27, 2023 12:12
@gandro gandro requested a review from squeed June 27, 2023 12:12
@gandro
Copy link
Member Author

gandro commented Jun 27, 2023

/test

@gandro gandro added the kind/cleanup This includes no functional changes. label Jun 27, 2023
@gandro
Copy link
Member Author

gandro commented Jun 27, 2023

ci-multicluster with connectivity test failed: timeout reached waiting lookup for localhost from pod cilium-test/client-6965d549d5-qbk6f in three jobs. I don't see anything obviously wrong in the sysdump: https://github.com/cilium/cilium/actions/runs/5390393415/jobs/9785687769

Restarting.

Edit: Created the following issue, since this seems to happen in other PRs too:

@gandro
Copy link
Member Author

gandro commented Jun 27, 2023

/ci-multicluster

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 27, 2023
@gandro gandro merged commit d1162cc into cilium:main Jun 27, 2023
65 of 66 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cni Impacts the Container Networking Interface between Cilium and the orchestrator. kind/cleanup This includes no functional changes. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants