Skip to content

Commit

Permalink
Merge pull request kubernetes#113994 from mengjiao-liu/contextual-log…
Browse files Browse the repository at this point in the history 
…ging-controller-certificates

certificate controller: use contextual logging
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Jun 21, 2023
2 parents b8d4eec + 5588e8a commit 28296ba
Show file tree
Hide file tree
Showing 10 changed files with 65 additions and 45 deletions.
9 changes: 5 additions & 4 deletions cmd/kube-controller-manager/app/certificates.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func startCSRSigningController(ctx context.Context, controllerContext Controller
certTTL := controllerContext.ComponentConfig.CSRSigningController.ClusterSigningDuration.Duration

if kubeletServingSignerCertFile, kubeletServingSignerKeyFile := getKubeletServingSignerFiles(controllerContext.ComponentConfig.CSRSigningController); len(kubeletServingSignerCertFile) > 0 || len(kubeletServingSignerKeyFile) > 0 {
kubeletServingSigner, err := signer.NewKubeletServingCSRSigningController(c, csrInformer, kubeletServingSignerCertFile, kubeletServingSignerKeyFile, certTTL)
kubeletServingSigner, err := signer.NewKubeletServingCSRSigningController(ctx, c, csrInformer, kubeletServingSignerCertFile, kubeletServingSignerKeyFile, certTTL)
if err != nil {
return nil, false, fmt.Errorf("failed to start kubernetes.io/kubelet-serving certificate controller: %v", err)
}
Expand All @@ -58,7 +58,7 @@ func startCSRSigningController(ctx context.Context, controllerContext Controller
}

if kubeletClientSignerCertFile, kubeletClientSignerKeyFile := getKubeletClientSignerFiles(controllerContext.ComponentConfig.CSRSigningController); len(kubeletClientSignerCertFile) > 0 || len(kubeletClientSignerKeyFile) > 0 {
kubeletClientSigner, err := signer.NewKubeletClientCSRSigningController(c, csrInformer, kubeletClientSignerCertFile, kubeletClientSignerKeyFile, certTTL)
kubeletClientSigner, err := signer.NewKubeletClientCSRSigningController(ctx, c, csrInformer, kubeletClientSignerCertFile, kubeletClientSignerKeyFile, certTTL)
if err != nil {
return nil, false, fmt.Errorf("failed to start kubernetes.io/kube-apiserver-client-kubelet certificate controller: %v", err)
}
Expand All @@ -68,7 +68,7 @@ func startCSRSigningController(ctx context.Context, controllerContext Controller
}

if kubeAPIServerSignerCertFile, kubeAPIServerSignerKeyFile := getKubeAPIServerClientSignerFiles(controllerContext.ComponentConfig.CSRSigningController); len(kubeAPIServerSignerCertFile) > 0 || len(kubeAPIServerSignerKeyFile) > 0 {
kubeAPIServerClientSigner, err := signer.NewKubeAPIServerClientCSRSigningController(c, csrInformer, kubeAPIServerSignerCertFile, kubeAPIServerSignerKeyFile, certTTL)
kubeAPIServerClientSigner, err := signer.NewKubeAPIServerClientCSRSigningController(ctx, c, csrInformer, kubeAPIServerSignerCertFile, kubeAPIServerSignerKeyFile, certTTL)
if err != nil {
return nil, false, fmt.Errorf("failed to start kubernetes.io/kube-apiserver-client certificate controller: %v", err)
}
Expand All @@ -78,7 +78,7 @@ func startCSRSigningController(ctx context.Context, controllerContext Controller
}

if legacyUnknownSignerCertFile, legacyUnknownSignerKeyFile := getLegacyUnknownSignerFiles(controllerContext.ComponentConfig.CSRSigningController); len(legacyUnknownSignerCertFile) > 0 || len(legacyUnknownSignerKeyFile) > 0 {
legacyUnknownSigner, err := signer.NewLegacyUnknownCSRSigningController(c, csrInformer, legacyUnknownSignerCertFile, legacyUnknownSignerKeyFile, certTTL)
legacyUnknownSigner, err := signer.NewLegacyUnknownCSRSigningController(ctx, c, csrInformer, legacyUnknownSignerCertFile, legacyUnknownSignerKeyFile, certTTL)
if err != nil {
return nil, false, fmt.Errorf("failed to start kubernetes.io/legacy-unknown certificate controller: %v", err)
}
Expand Down Expand Up @@ -150,6 +150,7 @@ func getLegacyUnknownSignerFiles(config csrsigningconfig.CSRSigningControllerCon

func startCSRApprovingController(ctx context.Context, controllerContext ControllerContext) (controller.Interface, bool, error) {
approver := approver.NewCSRApprovingController(
ctx,
controllerContext.ClientBuilder.ClientOrDie("certificate-controller"),
controllerContext.InformerFactory.Certificates().V1().CertificateSigningRequests(),
)
Expand Down
1 change: 0 additions & 1 deletion hack/logcheck.conf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ contextual k8s.io/kubernetes/test/e2e/dra/.*

# Most of kube-controller-manager has been converted, but not everything. At
# this point it is easier to list the exceptions.
-contextual k8s.io/kubernetes/pkg/controller/certificates/.*
-contextual k8s.io/kubernetes/pkg/controller/controller_ref_manager.go
-contextual k8s.io/kubernetes/pkg/controller/controller_utils.go
-contextual k8s.io/kubernetes/pkg/controller/deployment/.*
Expand Down
3 changes: 2 additions & 1 deletion pkg/controller/certificates/approver/sarapprove.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,13 @@ type sarApprover struct {
}

// NewCSRApprovingController creates a new CSRApprovingController.
func NewCSRApprovingController(client clientset.Interface, csrInformer certificatesinformers.CertificateSigningRequestInformer) *certificates.CertificateController {
func NewCSRApprovingController(ctx context.Context, client clientset.Interface, csrInformer certificatesinformers.CertificateSigningRequestInformer) *certificates.CertificateController {
approver := &sarApprover{
client: client,
recognizers: recognizers(),
}
return certificates.NewCertificateController(
ctx,
"csrapproving",
client,
csrInformer,
Expand Down
24 changes: 14 additions & 10 deletions pkg/controller/certificates/certificate_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,13 @@ type CertificateController struct {
}

func NewCertificateController(
ctx context.Context,
name string,
kubeClient clientset.Interface,
csrInformer certificatesinformers.CertificateSigningRequestInformer,
handler func(context.Context, *certificates.CertificateSigningRequest) error,
) *CertificateController {
logger := klog.FromContext(ctx)
cc := &CertificateController{
name: name,
kubeClient: kubeClient,
Expand All @@ -73,29 +75,29 @@ func NewCertificateController(
csrInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
AddFunc: func(obj interface{}) {
csr := obj.(*certificates.CertificateSigningRequest)
klog.V(4).Infof("Adding certificate request %s", csr.Name)
logger.V(4).Info("Adding certificate request", "csr", csr.Name)
cc.enqueueCertificateRequest(obj)
},
UpdateFunc: func(old, new interface{}) {
oldCSR := old.(*certificates.CertificateSigningRequest)
klog.V(4).Infof("Updating certificate request %s", oldCSR.Name)
logger.V(4).Info("Updating certificate request", "old", oldCSR.Name)
cc.enqueueCertificateRequest(new)
},
DeleteFunc: func(obj interface{}) {
csr, ok := obj.(*certificates.CertificateSigningRequest)
if !ok {
tombstone, ok := obj.(cache.DeletedFinalStateUnknown)
if !ok {
klog.V(2).Infof("Couldn't get object from tombstone %#v", obj)
logger.V(2).Info("Couldn't get object from tombstone", "object", obj)
return
}
csr, ok = tombstone.Obj.(*certificates.CertificateSigningRequest)
if !ok {
klog.V(2).Infof("Tombstone contained object that is not a CSR: %#v", obj)
logger.V(2).Info("Tombstone contained object that is not a CSR", "object", obj)
return
}
}
klog.V(4).Infof("Deleting certificate request %s", csr.Name)
logger.V(4).Info("Deleting certificate request", "csr", csr.Name)
cc.enqueueCertificateRequest(obj)
},
})
Expand All @@ -109,8 +111,9 @@ func (cc *CertificateController) Run(ctx context.Context, workers int) {
defer utilruntime.HandleCrash()
defer cc.queue.ShutDown()

klog.Infof("Starting certificate controller %q", cc.name)
defer klog.Infof("Shutting down certificate controller %q", cc.name)
logger := klog.FromContext(ctx)
logger.Info("Starting certificate controller", "name", cc.name)
defer logger.Info("Shutting down certificate controller", "name", cc.name)

if !cache.WaitForNamedCacheSync(fmt.Sprintf("certificate-%s", cc.name), ctx.Done(), cc.csrsSynced) {
return
Expand Down Expand Up @@ -142,7 +145,7 @@ func (cc *CertificateController) processNextWorkItem(ctx context.Context) bool {
if _, ignorable := err.(ignorableError); !ignorable {
utilruntime.HandleError(fmt.Errorf("Sync %v failed with : %v", cKey, err))
} else {
klog.V(4).Infof("Sync %v failed with : %v", cKey, err)
klog.FromContext(ctx).V(4).Info("Sync certificate request failed", "csr", cKey, "err", err)
}
return true
}
Expand All @@ -162,13 +165,14 @@ func (cc *CertificateController) enqueueCertificateRequest(obj interface{}) {
}

func (cc *CertificateController) syncFunc(ctx context.Context, key string) error {
logger := klog.FromContext(ctx)
startTime := time.Now()
defer func() {
klog.V(4).Infof("Finished syncing certificate request %q (%v)", key, time.Since(startTime))
logger.V(4).Info("Finished syncing certificate request", "csr", key, "elapsedTime", time.Since(startTime))
}()
csr, err := cc.csrLister.Get(key)
if errors.IsNotFound(err) {
klog.V(3).Infof("csr has been deleted: %v", key)
logger.V(3).Info("csr has been deleted", "csr", key)
return nil
}
if err != nil {
Expand Down
5 changes: 3 additions & 2 deletions pkg/controller/certificates/certificate_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,14 @@ import (
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes/fake"
"k8s.io/klog/v2/ktesting"
"k8s.io/kubernetes/pkg/controller"
)

// TODO flesh this out to cover things like not being able to find the csr in the cache, not
// auto-approving, etc.
func TestCertificateController(t *testing.T) {

_, ctx := ktesting.NewTestContext(t)
csr := &certificates.CertificateSigningRequest{
ObjectMeta: metav1.ObjectMeta{
Name: "test-csr",
Expand All @@ -55,6 +56,7 @@ func TestCertificateController(t *testing.T) {
}

controller := NewCertificateController(
ctx,
"test",
client,
informerFactory.Certificates().V1().CertificateSigningRequests(),
Expand All @@ -69,7 +71,6 @@ func TestCertificateController(t *testing.T) {
wait.PollUntil(10*time.Millisecond, func() (bool, error) {
return controller.queue.Len() >= 1, nil
}, stopCh)
ctx := context.TODO()
controller.processNextWorkItem(ctx)

actions := client.Actions()
Expand Down
33 changes: 18 additions & 15 deletions pkg/controller/certificates/cleaner/cleaner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,9 @@ func NewCSRCleanerController(
func (ccc *CSRCleanerController) Run(ctx context.Context, workers int) {
defer utilruntime.HandleCrash()

klog.Infof("Starting CSR cleaner controller")
defer klog.Infof("Shutting down CSR cleaner controller")
logger := klog.FromContext(ctx)
logger.Info("Starting CSR cleaner controller")
defer logger.Info("Shutting down CSR cleaner controller")

for i := 0; i < workers; i++ {
go wait.UntilWithContext(ctx, ccc.worker, pollingInterval)
Expand All @@ -91,20 +92,22 @@ func (ccc *CSRCleanerController) Run(ctx context.Context, workers int) {

// worker runs a thread that dequeues CSRs, handles them, and marks them done.
func (ccc *CSRCleanerController) worker(ctx context.Context) {
logger := klog.FromContext(ctx)
csrs, err := ccc.csrLister.List(labels.Everything())
if err != nil {
klog.Errorf("Unable to list CSRs: %v", err)
logger.Error(err, "Unable to list CSRs")
return
}
for _, csr := range csrs {
if err := ccc.handle(ctx, csr); err != nil {
klog.Errorf("Error while attempting to clean CSR %q: %v", csr.Name, err)
logger.Error(err, "Error while attempting to clean CSR", "csr", csr.Name)
}
}
}

func (ccc *CSRCleanerController) handle(ctx context.Context, csr *capi.CertificateSigningRequest) error {
if isIssuedPastDeadline(csr) || isDeniedPastDeadline(csr) || isFailedPastDeadline(csr) || isPendingPastDeadline(csr) || isIssuedExpired(csr) {
logger := klog.FromContext(ctx)
if isIssuedPastDeadline(logger, csr) || isDeniedPastDeadline(logger, csr) || isFailedPastDeadline(logger, csr) || isPendingPastDeadline(logger, csr) || isIssuedExpired(logger, csr) {
if err := ccc.csrClient.Delete(ctx, csr.Name, metav1.DeleteOptions{}); err != nil {
return fmt.Errorf("unable to delete CSR %q: %v", csr.Name, err)
}
Expand All @@ -114,10 +117,10 @@ func (ccc *CSRCleanerController) handle(ctx context.Context, csr *capi.Certifica

// isIssuedExpired checks if the CSR has been issued a certificate and if the
// expiration of the certificate (the NotAfter value) has passed.
func isIssuedExpired(csr *capi.CertificateSigningRequest) bool {
func isIssuedExpired(logger klog.Logger, csr *capi.CertificateSigningRequest) bool {
for _, c := range csr.Status.Conditions {
if c.Type == capi.CertificateApproved && isIssued(csr) && isExpired(csr) {
klog.Infof("Cleaning CSR %q as the associated certificate is expired.", csr.Name)
logger.Info("Cleaning CSR as the associated certificate is expired.", "csr", csr.Name)
return true
}
}
Expand All @@ -127,11 +130,11 @@ func isIssuedExpired(csr *capi.CertificateSigningRequest) bool {
// isPendingPastDeadline checks if the certificate has a Pending status and the
// creation time of the CSR is passed the deadline that pending requests are
// maintained for.
func isPendingPastDeadline(csr *capi.CertificateSigningRequest) bool {
func isPendingPastDeadline(logger klog.Logger, csr *capi.CertificateSigningRequest) bool {
// If there are no Conditions on the status, the CSR will appear via
// `kubectl` as `Pending`.
if len(csr.Status.Conditions) == 0 && isOlderThan(csr.CreationTimestamp, pendingExpiration) {
klog.Infof("Cleaning CSR %q as it is more than %v old and unhandled.", csr.Name, pendingExpiration)
logger.Info("Cleaning CSR as it is more than pendingExpiration duration old and unhandled.", "csr", csr.Name, "pendingExpiration", pendingExpiration)
return true
}
return false
Expand All @@ -140,10 +143,10 @@ func isPendingPastDeadline(csr *capi.CertificateSigningRequest) bool {
// isDeniedPastDeadline checks if the certificate has a Denied status and the
// creation time of the CSR is passed the deadline that denied requests are
// maintained for.
func isDeniedPastDeadline(csr *capi.CertificateSigningRequest) bool {
func isDeniedPastDeadline(logger klog.Logger, csr *capi.CertificateSigningRequest) bool {
for _, c := range csr.Status.Conditions {
if c.Type == capi.CertificateDenied && isOlderThan(c.LastUpdateTime, deniedExpiration) {
klog.Infof("Cleaning CSR %q as it is more than %v old and denied.", csr.Name, deniedExpiration)
logger.Info("Cleaning CSR as it is more than deniedExpiration duration old and denied.", "csr", csr.Name, "deniedExpiration", deniedExpiration)
return true
}
}
Expand All @@ -153,10 +156,10 @@ func isDeniedPastDeadline(csr *capi.CertificateSigningRequest) bool {
// isFailedPastDeadline checks if the certificate has a Failed status and the
// creation time of the CSR is passed the deadline that pending requests are
// maintained for.
func isFailedPastDeadline(csr *capi.CertificateSigningRequest) bool {
func isFailedPastDeadline(logger klog.Logger, csr *capi.CertificateSigningRequest) bool {
for _, c := range csr.Status.Conditions {
if c.Type == capi.CertificateFailed && isOlderThan(c.LastUpdateTime, deniedExpiration) {
klog.Infof("Cleaning CSR %q as it is more than %v old and failed.", csr.Name, deniedExpiration)
logger.Info("Cleaning CSR as it is more than deniedExpiration duration old and failed.", "csr", csr.Name, "deniedExpiration", deniedExpiration)
return true
}
}
Expand All @@ -166,10 +169,10 @@ func isFailedPastDeadline(csr *capi.CertificateSigningRequest) bool {
// isIssuedPastDeadline checks if the certificate has an Issued status and the
// creation time of the CSR is passed the deadline that issued requests are
// maintained for.
func isIssuedPastDeadline(csr *capi.CertificateSigningRequest) bool {
func isIssuedPastDeadline(logger klog.Logger, csr *capi.CertificateSigningRequest) bool {
for _, c := range csr.Status.Conditions {
if c.Type == capi.CertificateApproved && isIssued(csr) && isOlderThan(c.LastUpdateTime, approvedExpiration) {
klog.Infof("Cleaning CSR %q as it is more than %v old and approved.", csr.Name, approvedExpiration)
logger.Info("Cleaning CSR as it is more than approvedExpiration duration old and approved.", "csr", csr.Name, "approvedExpiration", approvedExpiration)
return true
}
}
Expand Down
7 changes: 4 additions & 3 deletions pkg/controller/certificates/rootcacertpublisher/publisher.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,9 @@ func (c *Publisher) Run(ctx context.Context, workers int) {
defer utilruntime.HandleCrash()
defer c.queue.ShutDown()

klog.Infof("Starting root CA certificate configmap publisher")
defer klog.Infof("Shutting down root CA certificate configmap publisher")
logger := klog.FromContext(ctx)
logger.Info("Starting root CA cert publisher controller")
defer logger.Info("Shutting down root CA cert publisher controller")

if !cache.WaitForNamedCacheSync("crt configmap", ctx.Done(), c.cmListerSynced) {
return
Expand Down Expand Up @@ -177,7 +178,7 @@ func (c *Publisher) syncNamespace(ctx context.Context, ns string) (err error) {
startTime := time.Now()
defer func() {
recordMetrics(startTime, err)
klog.V(4).Infof("Finished syncing namespace %q (%v)", ns, time.Since(startTime))
klog.FromContext(ctx).V(4).Info("Finished syncing namespace", "namespace", ns, "elapsedTime", time.Since(startTime))
}()

cm, err := c.cmLister.ConfigMaps(ns).Get(RootCACertConfigMapName)
Expand Down
14 changes: 10 additions & 4 deletions pkg/controller/certificates/signer/signer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,42 +44,47 @@ type CSRSigningController struct {
}

func NewKubeletServingCSRSigningController(
ctx context.Context,
client clientset.Interface,
csrInformer certificatesinformers.CertificateSigningRequestInformer,
caFile, caKeyFile string,
certTTL time.Duration,
) (*CSRSigningController, error) {
return NewCSRSigningController("csrsigning-kubelet-serving", capi.KubeletServingSignerName, client, csrInformer, caFile, caKeyFile, certTTL)
return NewCSRSigningController(ctx, "csrsigning-kubelet-serving", capi.KubeletServingSignerName, client, csrInformer, caFile, caKeyFile, certTTL)
}

func NewKubeletClientCSRSigningController(
ctx context.Context,
client clientset.Interface,
csrInformer certificatesinformers.CertificateSigningRequestInformer,
caFile, caKeyFile string,
certTTL time.Duration,
) (*CSRSigningController, error) {
return NewCSRSigningController("csrsigning-kubelet-client", capi.KubeAPIServerClientKubeletSignerName, client, csrInformer, caFile, caKeyFile, certTTL)
return NewCSRSigningController(ctx, "csrsigning-kubelet-client", capi.KubeAPIServerClientKubeletSignerName, client, csrInformer, caFile, caKeyFile, certTTL)
}

func NewKubeAPIServerClientCSRSigningController(
ctx context.Context,
client clientset.Interface,
csrInformer certificatesinformers.CertificateSigningRequestInformer,
caFile, caKeyFile string,
certTTL time.Duration,
) (*CSRSigningController, error) {
return NewCSRSigningController("csrsigning-kube-apiserver-client", capi.KubeAPIServerClientSignerName, client, csrInformer, caFile, caKeyFile, certTTL)
return NewCSRSigningController(ctx, "csrsigning-kube-apiserver-client", capi.KubeAPIServerClientSignerName, client, csrInformer, caFile, caKeyFile, certTTL)
}

func NewLegacyUnknownCSRSigningController(
ctx context.Context,
client clientset.Interface,
csrInformer certificatesinformers.CertificateSigningRequestInformer,
caFile, caKeyFile string,
certTTL time.Duration,
) (*CSRSigningController, error) {
return NewCSRSigningController("csrsigning-legacy-unknown", capiv1beta1.LegacyUnknownSignerName, client, csrInformer, caFile, caKeyFile, certTTL)
return NewCSRSigningController(ctx, "csrsigning-legacy-unknown", capiv1beta1.LegacyUnknownSignerName, client, csrInformer, caFile, caKeyFile, certTTL)
}

func NewCSRSigningController(
ctx context.Context,
controllerName string,
signerName string,
client clientset.Interface,
Expand All @@ -94,6 +99,7 @@ func NewCSRSigningController(

return &CSRSigningController{
certificateController: certificates.NewCertificateController(
ctx,
controllerName,
client,
csrInformer,
Expand Down
Loading

0 comments on commit 28296ba

Please sign in to comment.