From 13255900d6459e9acdad27d92ef8bef8d9274039 Mon Sep 17 00:00:00 2001 From: Serge Logvinov Date: Mon, 17 Jun 2024 10:10:33 +0300 Subject: [PATCH] refactor: contextual logging Migrate to contextual logging. Signed-off-by: Serge Logvinov --- cmd/talos-cloud-controller-manager/main.go | 17 +++++++++++------ pkg/certificatesigningrequest/controller.go | 18 +++++++++--------- pkg/talos/cloud.go | 12 ++++++------ pkg/talos/cloud_config.go | 2 +- pkg/talos/instances.go | 20 ++++++++++---------- 5 files changed, 37 insertions(+), 32 deletions(-) diff --git a/cmd/talos-cloud-controller-manager/main.go b/cmd/talos-cloud-controller-manager/main.go index d1cea3b..408904c 100644 --- a/cmd/talos-cloud-controller-manager/main.go +++ b/cmd/talos-cloud-controller-manager/main.go @@ -44,7 +44,8 @@ import ( func main() { ccmOptions, err := options.NewCloudControllerManagerOptions() if err != nil { - klog.Fatalf("unable to initialize command options: %v", err) + klog.ErrorS(err, "unable to initialize command options") + klog.FlushAndExit(klog.ExitFlushTimeout, 1) } fss := cliflag.NamedFlagSets{} @@ -53,7 +54,8 @@ func main() { command.Flags().VisitAll(func(flag *pflag.Flag) { if flag.Name == "cloud-provider" { if err := flag.Value.Set(talos.ProviderName); err != nil { - klog.Fatalf("unable to set cloud-provider flag value: %s", err) + klog.ErrorS(err, "unable to set cloud-provider flag value") + klog.FlushAndExit(klog.ExitFlushTimeout, 1) } } }) @@ -68,18 +70,21 @@ func cloudInitializer(config *config.CompletedConfig) cloudprovider.Interface { // initialize cloud provider with the cloud provider name and config file provided cloud, err := cloudprovider.InitCloudProvider(cloudConfig.Name, cloudConfig.CloudConfigFile) if err != nil { - klog.Fatalf("Cloud provider could not be initialized: %v", err) + klog.ErrorS(err, "Cloud provider could not be initialized") + klog.FlushAndExit(klog.ExitFlushTimeout, 1) } if cloud == nil { - klog.Fatalf("Cloud provider is nil") + klog.InfoS("Cloud provider is nil") + klog.FlushAndExit(klog.ExitFlushTimeout, 1) } if !cloud.HasClusterID() { if config.ComponentConfig.KubeCloudShared.AllowUntaggedCloud { - klog.Warning("detected a cluster without a ClusterID. A ClusterID will be required in the future. Please tag your cluster to avoid any future issues") + klog.InfoS("detected a cluster without a ClusterID. A ClusterID will be required in the future. Please tag your cluster to avoid any future issues") } else { - klog.Fatalf("no ClusterID found. A ClusterID is required for the cloud provider to function properly. This check can be bypassed by setting the allow-untagged-cloud option") + klog.InfoS("no ClusterID found. A ClusterID is required for the cloud provider to function properly. This check can be bypassed by setting the allow-untagged-cloud option") + klog.FlushAndExit(klog.ExitFlushTimeout, 1) } } diff --git a/pkg/certificatesigningrequest/controller.go b/pkg/certificatesigningrequest/controller.go index fbe8474..01dfc22 100644 --- a/pkg/certificatesigningrequest/controller.go +++ b/pkg/certificatesigningrequest/controller.go @@ -48,7 +48,7 @@ func (r *Reconciler) Run(ctx context.Context) { TimeoutSeconds: &watchTimeoutSeconds, // Default timeout: 20 minutes. }) if err != nil { - klog.Errorf("CertificateSigningRequestReconciler: failed to list CSR resources: %v", err) + klog.ErrorS(err, "CertificateSigningRequestReconciler: failed to list CSR resources") time.Sleep(10 * time.Second) // Pause for a while before retrying, otherwise we'll spam error logs. continue @@ -66,41 +66,41 @@ func (r *Reconciler) Run(ctx context.Context) { for { select { case <-ctx.Done(): - klog.V(4).Infof("CertificateSigningRequestReconciler: context canceled, terminating") + klog.V(4).InfoS("CertificateSigningRequestReconciler: context canceled, terminating") return case event, ok := <-csrWatcher.ResultChan(): if !ok { // Server timeout closed the watcher channel, loop again to re-create a new one. - klog.V(5).Infof("CertificateSigningRequestReconciler: API server closed watcher channel") + klog.V(5).InfoS("CertificateSigningRequestReconciler: API server closed watcher channel") break watch } csr, ok := event.Object.DeepCopyObject().(*certificatesv1.CertificateSigningRequest) if !ok { - klog.Errorf("CertificateSigningRequestReconciler: expected event of type *CertificateSigningRequest, got %v", - event.Object.GetObjectKind()) + klog.ErrorS(err, "CertificateSigningRequestReconciler: expected event of type *CertificateSigningRequest", + "kind", event.Object.GetObjectKind()) continue } valid, err := r.Reconcile(ctx, csr) if err != nil { - klog.Errorf("CertificateSigningRequestReconciler: failed to reconcile CSR %s: %v", csr.Name, err) + klog.ErrorS(err, "CertificateSigningRequestReconciler: failed to reconcile CSR", "name", csr.Name) continue } if _, err := r.kclient.CertificatesV1().CertificateSigningRequests().UpdateApproval(ctx, csr.Name, csr, metav1.UpdateOptions{}); err != nil { - klog.Errorf("CertificateSigningRequestReconciler: failed to approve/deny CSR %s: %v", csr.Name, err) + klog.ErrorS(err, "CertificateSigningRequestReconciler: failed to approve/deny CSR", "name", csr.Name) } if !valid { - klog.Warningf("CertificateSigningRequestReconciler: has been denied: %s", csr.Name) + klog.InfoS("CertificateSigningRequestReconciler: has been denied", "name", csr.Name) } else { - klog.V(3).Infof("CertificateSigningRequestReconciler: has been approved: %s", csr.Name) + klog.V(3).InfoS("CertificateSigningRequestReconciler: has been approved", "name", csr.Name) } } } diff --git a/pkg/talos/cloud.go b/pkg/talos/cloud.go index 3b601c6..51d6827 100644 --- a/pkg/talos/cloud.go +++ b/pkg/talos/cloud.go @@ -39,7 +39,7 @@ func init() { cloudprovider.RegisterCloudProvider(ProviderName, func(config io.Reader) (cloudprovider.Interface, error) { cfg, err := readCloudConfig(config) if err != nil { - klog.Errorf("failed to read config: %v", err) + klog.ErrorS(err, "failed to read config") return nil, err } @@ -69,14 +69,14 @@ func newCloud(config *cloudConfig) (cloudprovider.Interface, error) { func (c *cloud) Initialize(clientBuilder cloudprovider.ControllerClientBuilder, stop <-chan struct{}) { c.client.kclient = clientBuilder.ClientOrDie(ServiceAccountName) - klog.Infof("clientset initialized") + klog.InfoS("clientset initialized") ctx, cancel := context.WithCancel(context.Background()) c.ctx = ctx c.stop = cancel if err := c.client.refreshTalosClient(c.ctx); err != nil { - klog.Errorf("failed to initialized talos client: %v", err) + klog.ErrorS(err, "failed to initialized talos client") return } @@ -85,18 +85,18 @@ func (c *cloud) Initialize(clientBuilder cloudprovider.ControllerClientBuilder, // watching the provider's context for cancellation. go func(provider *cloud) { <-stop - klog.V(3).Infof("received cloud provider termination signal") + klog.V(3).InfoS("received cloud provider termination signal") provider.stop() }(c) if c.cfg.Global.ApproveNodeCSR { - klog.Infof("Started CSR Node controller") + klog.InfoS("Started CSR Node controller") c.csrController = certificatesigningrequest.NewCsrController(c.client.kclient, csrNodeChecks) go c.csrController.Run(c.ctx) } - klog.Infof("talos initialized") + klog.InfoS("talos initialized") } // LoadBalancer returns a balancer interface. diff --git a/pkg/talos/cloud_config.go b/pkg/talos/cloud_config.go index 52ecae2..2f6b402 100644 --- a/pkg/talos/cloud_config.go +++ b/pkg/talos/cloud_config.go @@ -44,7 +44,7 @@ func readCloudConfig(config io.Reader) (cloudConfig, error) { cfg.Global.Endpoints = strings.Split(endpoints, ",") } - klog.V(4).Infof("cloudConfig: %+v", cfg) + klog.V(4).InfoS("cloudConfig", "cfg", cfg) return cfg, nil } diff --git a/pkg/talos/instances.go b/pkg/talos/instances.go index 6433700..e863d0e 100644 --- a/pkg/talos/instances.go +++ b/pkg/talos/instances.go @@ -31,7 +31,7 @@ func newInstances(client *client) *instances { // InstanceExists returns true if the instance for the given node exists according to the cloud provider. // Use the node.name or node.spec.providerID field to find the node in the cloud provider. func (i *instances) InstanceExists(_ context.Context, node *v1.Node) (bool, error) { - klog.V(4).Info("instances.InstanceExists() called node: ", node.Name) + klog.V(4).InfoS("instances.InstanceExists() called", "node", klog.KRef("", node.Name)) return true, nil } @@ -39,7 +39,7 @@ func (i *instances) InstanceExists(_ context.Context, node *v1.Node) (bool, erro // InstanceShutdown returns true if the instance is shutdown according to the cloud provider. // Use the node.name or node.spec.providerID field to find the node in the cloud provider. func (i *instances) InstanceShutdown(_ context.Context, node *v1.Node) (bool, error) { - klog.V(4).Info("instances.InstanceShutdown() called, node: ", node.Name) + klog.V(4).InfoS("instances.InstanceShutdown() called", "node", klog.KRef("", node.Name)) return false, nil } @@ -50,7 +50,7 @@ func (i *instances) InstanceShutdown(_ context.Context, node *v1.Node) (bool, er // //nolint:gocyclo,cyclop func (i *instances) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloudprovider.InstanceMetadata, error) { - klog.V(4).Info("instances.InstanceMetadata() called, node: ", node.Name) + klog.V(4).InfoS("instances.InstanceMetadata() called", "node", klog.KRef("", node.Name)) if providedIP, ok := node.ObjectMeta.Annotations[cloudproviderapi.AnnotationAlphaProvidedIPAddr]; ok { nodeIPs := net.PreferedDualStackNodeIPs(i.c.config.Global.PreferIPv6, strings.Split(providedIP, ",")) @@ -75,14 +75,14 @@ func (i *instances) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloud break } - klog.Errorf("error getting metadata from the node %s: %v", node.Name, err) + klog.ErrorS(err, "error getting metadata from the node", "node", klog.KRef("", node.Name)) } if meta == nil { return nil, fmt.Errorf("error getting metadata from the node %s", node.Name) } - klog.V(5).Infof("instances.InstanceMetadata() resource: %+v", meta) + klog.V(5).InfoS("instances.InstanceMetadata()", "node", klog.KRef("", node.Name), "resource", meta) if meta.ProviderID == "" { meta.ProviderID = fmt.Sprintf("%s://%s/%s", ProviderName, meta.Platform, nodeIP) @@ -124,23 +124,23 @@ func (i *instances) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloud } if nodeSpec.Annotations != nil { - klog.V(4).Infof("instances.InstanceMetadata() node %s has annotations: %+v", node.Name, nodeSpec.Annotations) + klog.V(4).InfoS("instances.InstanceMetadata() node has annotations", "node", klog.KRef("", node.Name), "annotations", nodeSpec.Annotations) if err := syncNodeAnnotations(ctx, i.c, node, nodeSpec.Annotations); err != nil { - klog.Errorf("failed update annotations for node %s, %v", node.Name, err) + klog.ErrorS(err, "error updating annotations for the node", "node", klog.KRef("", node.Name)) } } nodeLabels := setTalosNodeLabels(i.c, meta) if nodeSpec.Labels != nil { - klog.V(4).Infof("instances.InstanceMetadata() node %s has labels: %+v", node.Name, nodeSpec.Labels) + klog.V(4).InfoS("instances.InstanceMetadata() node has labels", "node", klog.KRef("", node.Name), "labels", nodeSpec.Labels) maps.Copy(nodeLabels, nodeSpec.Labels) } if err := syncNodeLabels(i.c, node, nodeLabels); err != nil { - klog.Errorf("failed update labels for node %s, %v", node.Name, err) + klog.ErrorS(err, "error updating labels for the node", "node", klog.KRef("", node.Name)) } return &cloudprovider.InstanceMetadata{ @@ -152,7 +152,7 @@ func (i *instances) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloud }, nil } - klog.Warningf("instances.InstanceMetadata() is kubelet has --cloud-provider=external on the node %s?", node.Name) + klog.InfoS("instances.InstanceMetadata() is kubelet has args: --cloud-provider=external on the node?", node, klog.KRef("", node.Name)) return &cloudprovider.InstanceMetadata{}, nil }