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

Add server #10

Merged
merged 4 commits into from
Jul 28, 2022
Merged

Conversation

adrianchiris
Copy link
Collaborator

No description provided.

@adrianchiris
Copy link
Collaborator Author

Note: last 4 commits are relevant (rest will be removed when rebase)

@rollandf
Copy link
Collaborator

LGTM

@adrianchiris adrianchiris force-pushed the add-server branch 2 times, most recently from de22a97 to fbdf50d Compare July 26, 2022 06:54
if err != nil {
return "", fmt.Errorf("couldn't determine hostname: %v", err)
}
hostName = nodeName
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do you need the nodeName?
you can just
hostName , err := os.Hostname() and remove Line 33

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That would define a new hostName var in the inner scope (because of the :=)

i could do:

if len(hostName) == 0 {
  var err
  hostName, err = os.Hostname
....
}
...

it would save on temp var.

@ykulazhenkov
Copy link
Collaborator

ykulazhenkov commented Jul 27, 2022

General Q: Do we have/need clean-up logic to remove TC rules on POD removal?

add simple util method to return hostname

Signed-off-by: Adrian Chiris <[email protected]>
@adrianchiris
Copy link
Collaborator Author

General Q: Do we have/need clean-up logic to remove TC rules on POD removal?

Its tricky, since you need to be careful the VF is not re-allocated while deleting filters. (so you cannot do it during delete callback in server although operation is fairly short)

currently TC rules are aligned for all pods scheduled on node during sync loop.
if a pod interface is not associated with a multi-network-policy - we will remove TC filters from Rep.

to delete filters for interfaces that were allocated in the past and are not allocated at sync time we could maintain another map of deleted pods then process those first.

options stores Server option. user facing options
are exposed via flags

Signed-off-by: Adrian Chiris <[email protected]>
Server is the top level construct that ties various
components together.

It initializes informers and registers handlers for
the various objects. keeping track of changes of the various
objects as well as the current state of the cluster WRT
the tracked objects.

Main business logic is implemented in SyncMultiPolicy
method. which is invoked in a rate limited manner
to sync TC rules for representors currently associated
with pod interfaces scheduled on the current node

Signed-off-by: Adrian Chiris <[email protected]>
Run `go mod tidy` to update dependencies

Signed-off-by: Adrian Chiris <[email protected]>
Copy link
Collaborator

@ykulazhenkov ykulazhenkov left a comment

Choose a reason for hiding this comment

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

LGTM

@adrianchiris adrianchiris merged commit 8dbdbcd into k8snetworkplumbingwg:main Jul 28, 2022
Sign up for free to 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.

4 participants