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

Refactor logging statements in admission controller #7226

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions vertical-pod-autoscaler/pkg/admission-controller/certs.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,10 @@ type certsConfig struct {
func readFile(filePath string) []byte {
res, err := os.ReadFile(filePath)
if err != nil {
klog.Errorf("Error reading certificate file at %s: %v", filePath, err)
klog.ErrorS(err, "Error reading certificate file", "file", filePath)
return nil
}

klog.V(3).Infof("Successfully read %d bytes from %v", len(res), filePath)
klog.V(3).InfoS("Successfully read bytes from file", "bytes", len(res), "file", filePath)
return res
}

Expand Down Expand Up @@ -67,9 +66,9 @@ func (cr *certReloader) start(stop <-chan struct{}) error {
select {
case event := <-watcher.Events:
if event.Has(fsnotify.Create) || event.Has(fsnotify.Write) {
klog.V(2).Info("New certificate found, reloading")
klog.V(2).InfoS("New certificate found, reloading")
if err := cr.load(); err != nil {
klog.Errorf("Failed to reload certificate: %s", err)
klog.ErrorS(err, "Failed to reload certificate")
}
}
case err := <-watcher.Errors:
Expand Down
2 changes: 1 addition & 1 deletion vertical-pod-autoscaler/pkg/admission-controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func main() {
var limitRangeCalculator limitrange.LimitRangeCalculator
limitRangeCalculator, err := limitrange.NewLimitsRangeCalculator(factory)
if err != nil {
klog.Errorf("Failed to create limitRangeCalculator, falling back to not checking limits. Error message: %s", err)
klog.ErrorS(err, "Failed to create limitRangeCalculator, falling back to not checking limits.")
limitRangeCalculator = limitrange.NewNoopLimitsCalculator()
}
recommendationProvider := recommendation.NewProvider(limitRangeCalculator, vpa_api_util.NewCappingRecommendationProcessor(limitRangeCalculator))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,10 @@ func (h *resourceHandler) GetPatches(ctx context.Context, ar *admissionv1.Admiss
pod.Name = pod.GenerateName + "%"
pod.Namespace = namespace
}
klog.V(4).Infof("Admitting pod %s", klog.KObj(&pod))
klog.V(4).InfoS("Admitting pod", "pod", klog.KObj(&pod))
controllingVpa := h.vpaMatcher.GetMatchingVPA(ctx, &pod)
if controllingVpa == nil {
klog.V(4).Infof("No matching VPA found for pod %s", klog.KObj(&pod))
klog.V(4).InfoS("No matching VPA found for pod", "pod", klog.KObj(&pod))
return []resource_admission.PatchRecord{}, nil
}
pod, err := h.preProcessor.Process(pod)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@ func GetContainersResources(pod *core.Pod, vpaResourcePolicy *vpa_types.PodResou
recommendation := vpa_api_util.GetRecommendationForContainer(container.Name, &podRecommendation)
if recommendation == nil {
if !addAll {
klog.V(2).Infof("no matching recommendation found for container %s, skipping", container.Name)
klog.V(2).InfoS("No recommendation found for container, skipping", "container", container.Name)
continue
}
klog.V(2).Infof("no matching recommendation found for container %s, using Pod request", container.Name)
klog.V(2).InfoS("No match found for container, using Pod request", "container", container.Name)
resources[i].Requests = container.Resources.Requests
} else {
resources[i].Requests = recommendation.Target
Expand Down Expand Up @@ -106,10 +106,10 @@ func GetContainersResources(pod *core.Pod, vpaResourcePolicy *vpa_types.PodResou
// The returned slice corresponds 1-1 to containers in the Pod.
func (p *recommendationProvider) GetContainersResourcesForPod(pod *core.Pod, vpa *vpa_types.VerticalPodAutoscaler) ([]vpa_api_util.ContainerResources, vpa_api_util.ContainerToAnnotationsMap, error) {
if vpa == nil || pod == nil {
klog.V(2).Infof("can't calculate recommendations, one of vpa(%+v), pod(%+v) is nil", vpa, pod)
klog.V(2).InfoS("Can't calculate recommendations, one of VPA or Pod is nil", "vpa", vpa, "pod", pod)
return nil, nil, nil
}
klog.V(2).Infof("updating requirements for pod %s.", pod.Name)
klog.V(2).InfoS("Updating requirements for pod", "pod", pod.Name)

var annotations vpa_api_util.ContainerToAnnotationsMap
recommendedPodResources := &vpa_types.RecommendedPodResources{}
Expand All @@ -118,7 +118,7 @@ func (p *recommendationProvider) GetContainersResourcesForPod(pod *core.Pod, vpa
var err error
recommendedPodResources, annotations, err = p.recommendationProcessor.Apply(vpa.Status.Recommendation, vpa.Spec.ResourcePolicy, vpa.Status.Conditions, pod)
if err != nil {
klog.V(2).Infof("cannot process recommendation for pod %s", klog.KObj(pod))
klog.V(2).InfoS("Cannot process recommendation for pod", "pod", klog.KObj(pod))
return nil, annotations, err
}
}
Expand All @@ -135,7 +135,7 @@ func (p *recommendationProvider) GetContainersResourcesForPod(pod *core.Pod, vpa
// Ensure that we are not propagating empty resource key if any.
for _, resource := range containerResources {
if resource.RemoveEmptyResourceKeyIfAny() {
klog.Infof("An empty resource key was found and purged for pod=%s with vpa=%s", klog.KObj(pod), klog.KObj(vpa))
klog.InfoS("An empty resource key was found and purged", "pod", klog.KObj(pod), "vpa", klog.KObj(vpa))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (h *resourceHandler) GetPatches(_ context.Context, ar *v1.AdmissionRequest)
return nil, err
}

klog.V(4).Infof("Processing vpa: %v", vpa)
klog.V(4).InfoS("Processing vpa", "vpa", vpa)
patches := []resource.PatchRecord{}
if vpa.Spec.UpdatePolicy == nil {
// Sets the default updatePolicy.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func NewMatcher(vpaLister vpa_lister.VerticalPodAutoscalerLister,
func (m *matcher) GetMatchingVPA(ctx context.Context, pod *core.Pod) *vpa_types.VerticalPodAutoscaler {
configs, err := m.vpaLister.VerticalPodAutoscalers(pod.Namespace).List(labels.Everything())
if err != nil {
klog.Errorf("failed to get vpa configs: %v", err)
klog.ErrorS(err, "Failed to get vpa configs")
return nil
}
onConfigs := make([]*vpa_api_util.VpaWithSelector, 0)
Expand All @@ -64,15 +64,15 @@ func (m *matcher) GetMatchingVPA(ctx context.Context, pod *core.Pod) *vpa_types.
}
selector, err := m.selectorFetcher.Fetch(ctx, vpaConfig)
if err != nil {
klog.V(3).Infof("skipping VPA object %s because we cannot fetch selector: %s", klog.KObj(vpaConfig), err)
klog.V(3).InfoS("Skipping VPA object because we cannot fetch selector", "vpa", klog.KObj(vpaConfig), "error", err)
continue
}
onConfigs = append(onConfigs, &vpa_api_util.VpaWithSelector{
Vpa: vpaConfig,
Selector: selector,
})
}
klog.V(2).Infof("Let's choose from %d configs for pod %s", len(onConfigs), klog.KObj(pod))
klog.V(2).InfoS("Let's choose from", "configs", len(onConfigs), "pod", klog.KObj(pod))
result := vpa_api_util.GetControllingVPAForPod(ctx, pod, onConfigs, m.controllerFetcher)
if result != nil {
return result.Vpa
Expand Down
Loading