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 logging for policy creation and deletion events #1445

Merged
merged 3 commits into from Jan 7, 2021

Conversation

lengrongfu
Copy link
Contributor

Related issue

closes #1264

What type of PR is this?

/kind feature

Proposed changes

Checklist

.gitignore Outdated
@@ -9,3 +9,4 @@ cmd/initContainer/kyvernopre
cmd/kyverno/kyverno
cmd/cli/kubectl-kyverno/kyverno
kubectl-kyverno
vendor/
Copy link
Member

Choose a reason for hiding this comment

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

We do not have vendor in Kyverno repo, please remove it.

logger.V(4).Info("deleting policy", "name", p.Name)
logger.Info("deleting policy event", "uid", p.UID, "kind", "ClusterPolicy", "policy_name", p.Name)

//logger.V(4).Info("deleting policy", "name", p.Name)
Copy link
Member

Choose a reason for hiding this comment

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

Please remove commented log.

@@ -235,6 +240,9 @@ func (pc *PolicyController) deletePolicy(obj interface{}) {
func (pc *PolicyController) addNsPolicy(obj interface{}) {
logger := pc.log
p := obj.(*kyverno.Policy)

logger.Info("creating ns policy event", "uid", p.UID, "kind", "ClusterPolicy", "policy_name", p.Name, "namespaces", p.Namespace)
Copy link
Member

Choose a reason for hiding this comment

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

kind should be Policy.

pol := ConvertPolicyToClusterPolicy(p)

logger.Info("deleting ns policy event", "uid", pol.UID, "kind", "ClusterPolicy", "policy_name", pol.Name, "namespaces", pol.Namespace)
Copy link
Member

Choose a reason for hiding this comment

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

kind=Policy.

pol := ConvertPolicyToClusterPolicy(p)

logger.Info("deleting ns policy event", "uid", pol.UID, "kind", "ClusterPolicy", "policy_name", pol.Name, "namespaces", pol.Namespace)

logger.V(4).Info("deleting namespace policy", "namespace", pol.Namespace, "name", pol.Name)
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove duplicate log.

@@ -180,6 +180,9 @@ func (pc *PolicyController) canBackgroundProcess(p *kyverno.ClusterPolicy) bool
func (pc *PolicyController) addPolicy(obj interface{}) {
logger := pc.log
p := obj.(*kyverno.ClusterPolicy)

logger.Info("creating policy event", "uid", p.UID, "kind", "ClusterPolicy", "policy_name", p.Name)
Copy link
Member

Choose a reason for hiding this comment

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

Let's change the message to "policy created".

@@ -224,7 +227,9 @@ func (pc *PolicyController) deletePolicy(obj interface{}) {
}
}

logger.V(4).Info("deleting policy", "name", p.Name)
logger.Info("deleting policy event", "uid", p.UID, "kind", "ClusterPolicy", "policy_name", p.Name)
Copy link
Member

Choose a reason for hiding this comment

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

Same here - "policy deleted".

@@ -180,6 +180,9 @@ func (pc *PolicyController) canBackgroundProcess(p *kyverno.ClusterPolicy) bool
func (pc *PolicyController) addPolicy(obj interface{}) {
logger := pc.log
p := obj.(*kyverno.ClusterPolicy)

logger.Info("policy created event", "uid", p.UID, "kind", "Policy", "policy_name", p.Name)
Copy link
Member

Choose a reason for hiding this comment

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

I should make it clearer in the last comment. In Kyverno, we have ClusterPolicy and Policy, similar to ClusterRole and Role, the ClusterPolicy is a cluster-wide resource and the Policy is a namespaced source.

So here the kind should be ClustserPolicy, or you can use p.Kind to get the kind.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I first started, I used p.Kind to get kind, but I found that what I got was empty, so I changed it to a fixed value. I understand what you mean, I will modify it again according to the correct kind type.

@realshuting realshuting merged commit fab777c into kyverno:main Jan 7, 2021
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.

Better logging for policy creation and deletion events
2 participants