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

Add support for the number of networkpolicies per node #834

Merged
merged 1 commit into from
Jun 19, 2020
Merged

Add support for the number of networkpolicies per node #834

merged 1 commit into from
Jun 19, 2020

Conversation

yktsubo
Copy link
Contributor

@yktsubo yktsubo commented Jun 13, 2020 •
edited
Loading

  • Change name of networkpolicy rule metric variable
  • Add support the number of networkpolicies per node

This PR is a part of #713 feature request

Signed-off-by: Yuki Tsuboi ytsuboi@vmware.com

@antrea-bot
Copy link
Collaborator

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

  • /test-e2e: to trigger e2e tests.
  • /skip-e2e: to skip e2e tests.
  • /test-conformance: to trigger conformance tests.
  • /skip-conformance: to skip conformance tests.
  • /test-networkpolicy: to trigger networkpolicy tests.
  • /skip-networkpolicy: to skip networkpolicy tests.
  • /test-windows-conformance: to trigger windows conformance tests.
  • /skip-windows-conformance: to skip windows conformance tests.
  • /test-all: to trigger all tests.
  • /skip-all: to skip all tests.

These commands can only be run by members of the vmware-tanzu organization.

tnqn
tnqn reviewed Jun 15, 2020
View reviewed changes
pkg/agent/controller/networkpolicy/reconciler.go Outdated
@@ -257,9 +257,9 @@ func (r *reconciler) add(rule *CompletedRule) error {

// Count up antrea_agent_ingress_networkpolicy_rule_count or antrea_agent_egress_networkpolicy_rule_count
if rule.Direction == v1beta1.DirectionIn {
metrics.IngressNetworkPolicyCount.Inc()
metrics.IngressNetworkPolicyRuleCount.Inc()
Copy link
Member

Choose a reason for hiding this comment

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

I feel IngressNetworkPolicyRuleCount and EgressNetworkPolicyRuleCount could also be tracked in cache.go together with NetworkPolicyCount as that's the source of reconciler.

Copy link
Member

Choose a reason for hiding this comment

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

+1 Putting them together also makes it easily maintainable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the comment.It sounds good. Let me see

@yktsubo yktsubo force-pushed the add_networkpolicy_per_node_metrics branch from 05562c8 to 3a84c27 Compare June 16, 2020 15:20
tnqn
tnqn previously approved these changes Jun 16, 2020
View reviewed changes
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@srikartati
Copy link
Member

/test-all

srikartati
srikartati previously approved these changes Jun 16, 2020
View reviewed changes
Copy link
Member

@srikartati srikartati left a comment

Choose a reason for hiding this comment

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

LGTM

@srikartati
Copy link
Member

/test-e2e

antoninbas
antoninbas reviewed Jun 17, 2020
View reviewed changes
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

@yktsubo can you squash your commits and fix the following typo in the commit title:

s/Add support the number of networkpolicies per node/Add support for the number of networkpolicies per node

I would actually suggest to use this title for the commit instead: "Add new Prometheus metric for total number of NetworkPolicies per Node"

@yktsubo yktsubo changed the title Add support the number of networkpolicies per node Add support for the number of networkpolicies per node Jun 17, 2020
Add support for the number of networkpolicies per node (antrea-io#834)
d1d9486 
Signed-off-by: Yuki Tsuboi <ytsuboi@vmware.com>
@yktsubo yktsubo dismissed stale reviews from srikartati and tnqn via d1d9486 June 17, 2020 12:34
@yktsubo yktsubo force-pushed the add_networkpolicy_per_node_metrics branch from 3a84c27 to d1d9486 Compare June 17, 2020 12:34
@yktsubo
Copy link
Contributor Author

yktsubo commented Jun 17, 2020 •
edited
Loading

Hi @antoninbas
Thank you for pointing out. I updated this PR title and squashed commits as you suggested.

@tnqn
Copy link
Member

tnqn commented Jun 17, 2020

/test-all

1 similar comment
@yktsubo
Copy link
Contributor Author

yktsubo commented Jun 18, 2020

/test-all

@tnqn
Copy link
Member

tnqn commented Jun 19, 2020

/skip-whole-conformance
/test-networkpolicy

@tnqn
Copy link
Member

tnqn commented Jun 19, 2020

Merging, thanks @yktsubo for your work.

@tnqn tnqn merged commit 2634d0d into antrea-io:master Jun 19, 2020
@yktsubo
Copy link
Contributor Author

yktsubo commented Jun 19, 2020

All, Thank you for your support until merging.

@yktsubo yktsubo deleted the add_networkpolicy_per_node_metrics branch June 19, 2020 12:19
GraysonWu pushed a commit to GraysonWu/antrea that referenced this pull request Sep 22, 2020
@GraysonWu
Add support for the number of networkpolicies per node (antrea-io#834)
351d242 
Signed-off-by: Yuki Tsuboi <ytsuboi@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antoninbas antoninbas antoninbas left review comments

@tnqn tnqn tnqn approved these changes

@srikartati srikartati srikartati left review comments

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

Successfully merging this pull request may close these issues.
6 participants
@yktsubo @antrea-bot @srikartati @tnqn @antoninbas @vmwclabot