From d8a76e9b8a3934ca03c073c2c2bdf59b04894473 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Mon, 15 Apr 2024 10:11:32 -0400 Subject: [PATCH] OCPBUGS-35305: [release-4.15] catalog-operator: delete catalog pods stuck in Terminating state due to unreachable node Signed-off-by: Joe Lanford Upstream-repository: operator-lifecycle-manager Upstream-commit: 68c24cf94371f7a73619966f9e49612fd16fa76a Signed-off-by: Per Goncalves da Silva --- go.mod | 2 +- go.sum | 4 +- staging/operator-lifecycle-manager/Dockerfile | 4 +- staging/operator-lifecycle-manager/go.mod | 2 +- staging/operator-lifecycle-manager/go.sum | 4 +- .../controller/registry/reconciler/grpc.go | 83 ++++- .../controller/registry/reconciler/grpc.go | 83 ++++- vendor/k8s.io/utils/integer/integer.go | 8 +- vendor/k8s.io/utils/pointer/pointer.go | 283 ++++-------------- vendor/k8s.io/utils/ptr/OWNERS | 10 + vendor/k8s.io/utils/ptr/README.md | 3 + vendor/k8s.io/utils/ptr/ptr.go | 73 +++++ vendor/k8s.io/utils/trace/trace.go | 21 +- vendor/modules.txt | 3 +- 14 files changed, 315 insertions(+), 268 deletions(-) create mode 100644 vendor/k8s.io/utils/ptr/OWNERS create mode 100644 vendor/k8s.io/utils/ptr/README.md create mode 100644 vendor/k8s.io/utils/ptr/ptr.go diff --git a/go.mod b/go.mod index c2720548c9..b1a90241c2 100644 --- a/go.mod +++ b/go.mod @@ -28,7 +28,7 @@ require ( k8s.io/client-go v0.27.7 k8s.io/code-generator v0.27.7 k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f - k8s.io/utils v0.0.0-20230220204549-a5ecb0141aa5 + k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0 sigs.k8s.io/controller-runtime v0.15.0 sigs.k8s.io/controller-tools v0.8.0 ) diff --git a/go.sum b/go.sum index c38ffcea03..630b7800e9 100644 --- a/go.sum +++ b/go.sum @@ -1479,8 +1479,8 @@ k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f/go.mod h1:byini6yhqGC14c3 k8s.io/kubectl v0.27.7 h1:HTEDa4s/oWjB3t5ysdW1yKlcNl9bzigcqWBq0LIIe3k= k8s.io/kubectl v0.27.7/go.mod h1:Xb1Ubc8uN1i2RvSN1HCgSHTtzgX0woihMk/gW7XbjJU= k8s.io/utils v0.0.0-20200324210504-a9aa75ae1b89/go.mod h1:sZAwmy6armz5eXlNoLmJcl4F1QuKu7sr+mFQ0byX7Ew= -k8s.io/utils v0.0.0-20230220204549-a5ecb0141aa5 h1:kmDqav+P+/5e1i9tFfHq1qcF3sOrDp+YEkVDAHu7Jwk= -k8s.io/utils v0.0.0-20230220204549-a5ecb0141aa5/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= +k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0 h1:jgGTlFYnhF1PM1Ax/lAlxUPE+KfCIXHaathvJg1C3ak= +k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= oras.land/oras-go v1.2.4 h1:djpBY2/2Cs1PV87GSJlxv4voajVOMZxqqtq9AB8YNvY= oras.land/oras-go v1.2.4/go.mod h1:DYcGfb3YF1nKjcezfX2SNlDAeQFKSXmf+qrFmrh4324= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= diff --git a/staging/operator-lifecycle-manager/Dockerfile b/staging/operator-lifecycle-manager/Dockerfile index 1ed3c19c3d..c4bc9e22b9 100644 --- a/staging/operator-lifecycle-manager/Dockerfile +++ b/staging/operator-lifecycle-manager/Dockerfile @@ -2,11 +2,11 @@ FROM quay.io/fedora/fedora:37-x86_64 as builder LABEL stage=builder WORKDIR /build -# install dependencies and go 1.16 +# install dependencies and go 1.21 # copy just enough of the git repo to parse HEAD, used to record version in OLM binaries RUN dnf update -y && dnf install -y bash make git mercurial jq wget && dnf upgrade -y -RUN curl -sSL https://go.dev/dl/go1.20.linux-amd64.tar.gz | tar -xzf - -C /usr/local +RUN curl -sSL https://go.dev/dl/go1.21.9.linux-amd64.tar.gz | tar -xzf - -C /usr/local ENV PATH=/usr/local/go/bin:$PATH COPY .git/HEAD .git/HEAD COPY .git/refs/heads/. .git/refs/heads diff --git a/staging/operator-lifecycle-manager/go.mod b/staging/operator-lifecycle-manager/go.mod index db99795914..fd8ec99a70 100644 --- a/staging/operator-lifecycle-manager/go.mod +++ b/staging/operator-lifecycle-manager/go.mod @@ -52,7 +52,7 @@ require ( k8s.io/klog/v2 v2.90.1 k8s.io/kube-aggregator v0.25.3 k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f - k8s.io/utils v0.0.0-20230220204549-a5ecb0141aa5 + k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0 sigs.k8s.io/controller-runtime v0.15.0 sigs.k8s.io/controller-tools v0.8.0 sigs.k8s.io/kind v0.20.0 diff --git a/staging/operator-lifecycle-manager/go.sum b/staging/operator-lifecycle-manager/go.sum index f605fe87fc..dd9855be73 100644 --- a/staging/operator-lifecycle-manager/go.sum +++ b/staging/operator-lifecycle-manager/go.sum @@ -1345,8 +1345,8 @@ k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f h1:2kWPakN3i/k81b0gvD5C5F k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f/go.mod h1:byini6yhqGC14c3ebc/QwanvYwhuMWF6yz2F8uwW8eg= k8s.io/kubectl v0.27.7 h1:HTEDa4s/oWjB3t5ysdW1yKlcNl9bzigcqWBq0LIIe3k= k8s.io/kubectl v0.27.7/go.mod h1:Xb1Ubc8uN1i2RvSN1HCgSHTtzgX0woihMk/gW7XbjJU= -k8s.io/utils v0.0.0-20230220204549-a5ecb0141aa5 h1:kmDqav+P+/5e1i9tFfHq1qcF3sOrDp+YEkVDAHu7Jwk= -k8s.io/utils v0.0.0-20230220204549-a5ecb0141aa5/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= +k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0 h1:jgGTlFYnhF1PM1Ax/lAlxUPE+KfCIXHaathvJg1C3ak= +k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= oras.land/oras-go v1.2.4 h1:djpBY2/2Cs1PV87GSJlxv4voajVOMZxqqtq9AB8YNvY= oras.land/oras-go v1.2.4/go.mod h1:DYcGfb3YF1nKjcezfX2SNlDAeQFKSXmf+qrFmrh4324= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= diff --git a/staging/operator-lifecycle-manager/pkg/controller/registry/reconciler/grpc.go b/staging/operator-lifecycle-manager/pkg/controller/registry/reconciler/grpc.go index e124bb07d3..f13aa17798 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/registry/reconciler/grpc.go +++ b/staging/operator-lifecycle-manager/pkg/controller/registry/reconciler/grpc.go @@ -2,19 +2,22 @@ package reconciler import ( "context" + "errors" "fmt" + "strings" "time" "github.com/google/go-cmp/cmp" "github.com/operator-framework/api/pkg/operators/v1alpha1" - "github.com/pkg/errors" + pkgerrors "github.com/pkg/errors" "github.com/sirupsen/logrus" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install" controllerclient "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/controller-runtime/client" @@ -262,7 +265,7 @@ func (c *GrpcRegistryReconciler) EnsureRegistryServer(logger *logrus.Entry, cata //TODO: if any of these error out, we should write a status back (possibly set RegistryServiceStatus to nil so they get recreated) sa, err := c.ensureSA(source) if err != nil && !apierrors.IsAlreadyExists(err) { - return errors.Wrapf(err, "error ensuring service account: %s", source.GetName()) + return pkgerrors.Wrapf(err, "error ensuring service account: %s", source.GetName()) } sa, err = c.OpClient.GetServiceAccount(sa.GetNamespace(), sa.GetName()) @@ -285,20 +288,20 @@ func (c *GrpcRegistryReconciler) EnsureRegistryServer(logger *logrus.Entry, cata return err } if err := c.ensurePod(logger, source, sa, overwritePod); err != nil { - return errors.Wrapf(err, "error ensuring pod: %s", pod.GetName()) + return pkgerrors.Wrapf(err, "error ensuring pod: %s", pod.GetName()) } if err := c.ensureUpdatePod(logger, sa, source); err != nil { if _, ok := err.(UpdateNotReadyErr); ok { return err } - return errors.Wrapf(err, "error ensuring updated catalog source pod: %s", pod.GetName()) + return pkgerrors.Wrapf(err, "error ensuring updated catalog source pod: %s", pod.GetName()) } service, err := source.Service() if err != nil { return err } if err := c.ensureService(source, overwrite); err != nil { - return errors.Wrapf(err, "error ensuring service: %s", service.GetName()) + return pkgerrors.Wrapf(err, "error ensuring service: %s", service.GetName()) } if overwritePod { @@ -338,16 +341,41 @@ func isRegistryServiceStatusValid(source *grpcCatalogSourceDecorator) (bool, err } func (c *GrpcRegistryReconciler) ensurePod(logger *logrus.Entry, source grpcCatalogSourceDecorator, serviceAccount *corev1.ServiceAccount, overwrite bool) error { - // currentLivePods refers to the currently live instances of the catalog source - currentLivePods := c.currentPods(logger, source) - if len(currentLivePods) > 0 { + // currentPods refers to the current pod instances of the catalog source + currentPods := c.currentPods(logger, source) + + var forceDeleteErrs []error + // Remove dead pods from the slice without allocating a new slice + // See https://go.dev/wiki/SliceTricks#filtering-without-allocating + tmpSlice := currentPods[:0] + for _, pod := range currentPods { + if !isPodDead(pod) { + logger.WithFields(logrus.Fields{"pod.namespace": source.GetNamespace(), "pod.name": pod.GetName()}).Debug("pod is alive") + tmpSlice = append(tmpSlice, pod) + continue + } + + logger.WithFields(logrus.Fields{"pod.namespace": source.GetNamespace(), "pod.name": pod.GetName()}).Info("force deleting dead pod") + if err := c.OpClient.KubernetesInterface().CoreV1().Pods(source.GetNamespace()).Delete(context.TODO(), pod.GetName(), metav1.DeleteOptions{ + GracePeriodSeconds: ptr.To[int64](0), + }); err != nil && !apierrors.IsNotFound(err) { + forceDeleteErrs = append(forceDeleteErrs, pkgerrors.Wrapf(err, "error deleting old pod: %s", pod.GetName())) + } + } + currentPods = tmpSlice + + if len(forceDeleteErrs) > 0 { + return errors.Join(forceDeleteErrs...) + } + + if len(currentPods) > 0 { if !overwrite { return nil } - for _, p := range currentLivePods { + for _, p := range currentPods { logger.WithFields(logrus.Fields{"pod.namespace": source.GetNamespace(), "pod.name": p.GetName()}).Info("deleting current pod") if err := c.OpClient.KubernetesInterface().CoreV1().Pods(source.GetNamespace()).Delete(context.TODO(), p.GetName(), *metav1.NewDeleteOptions(1)); err != nil && !apierrors.IsNotFound(err) { - return errors.Wrapf(err, "error deleting old pod: %s", p.GetName()) + return pkgerrors.Wrapf(err, "error deleting old pod: %s", p.GetName()) } } } @@ -358,7 +386,7 @@ func (c *GrpcRegistryReconciler) ensurePod(logger *logrus.Entry, source grpcCata logger.WithFields(logrus.Fields{"pod.namespace": source.GetNamespace(), "pod.name": desiredPod.Namespace}).Info("deleting current pod") _, err = c.OpClient.KubernetesInterface().CoreV1().Pods(source.GetNamespace()).Create(context.TODO(), desiredPod, metav1.CreateOptions{}) if err != nil { - return errors.Wrapf(err, "error creating new pod: %s", desiredPod.GetGenerateName()) + return pkgerrors.Wrapf(err, "error creating new pod: %s", desiredPod.GetGenerateName()) } return nil @@ -378,7 +406,7 @@ func (c *GrpcRegistryReconciler) ensureUpdatePod(logger *logrus.Entry, serviceAc logger.Infof("catalog update required at %s", time.Now().String()) pod, err := c.createUpdatePod(source, serviceAccount) if err != nil { - return errors.Wrapf(err, "creating update catalog source pod") + return pkgerrors.Wrapf(err, "creating update catalog source pod") } source.SetLastUpdateTime() return UpdateNotReadyErr{catalogName: source.GetName(), podName: pod.GetName()} @@ -410,7 +438,7 @@ func (c *GrpcRegistryReconciler) ensureUpdatePod(logger *logrus.Entry, serviceAc for _, p := range currentLivePods { logger.WithFields(logrus.Fields{"live-pod.namespace": source.GetNamespace(), "live-pod.name": p.Name}).Info("deleting current live pods") if err := c.OpClient.KubernetesInterface().CoreV1().Pods(source.GetNamespace()).Delete(context.TODO(), p.GetName(), *metav1.NewDeleteOptions(1)); err != nil && !apierrors.IsNotFound(err) { - return errors.Wrapf(errors.Wrapf(err, "error deleting pod: %s", p.GetName()), "detected imageID change: error deleting old catalog source pod") + return pkgerrors.Wrapf(pkgerrors.Wrapf(err, "error deleting pod: %s", p.GetName()), "detected imageID change: error deleting old catalog source pod") } } // done syncing @@ -420,7 +448,7 @@ func (c *GrpcRegistryReconciler) ensureUpdatePod(logger *logrus.Entry, serviceAc // delete update pod right away, since the digest match, to prevent long-lived duplicate catalog pods logger.WithFields(logrus.Fields{"update-pod.namespace": updatePod.Namespace, "update-pod.name": updatePod.Name}).Debug("catalog polling result: no update; removing duplicate update pod") if err := c.OpClient.KubernetesInterface().CoreV1().Pods(source.GetNamespace()).Delete(context.TODO(), updatePod.GetName(), *metav1.NewDeleteOptions(1)); err != nil && !apierrors.IsNotFound(err) { - return errors.Wrapf(errors.Wrapf(err, "error deleting pod: %s", updatePod.GetName()), "duplicate catalog polling pod") + return pkgerrors.Wrapf(pkgerrors.Wrapf(err, "error deleting pod: %s", updatePod.GetName()), "duplicate catalog polling pod") } } @@ -523,6 +551,29 @@ func imageChanged(logger *logrus.Entry, updatePod *corev1.Pod, servingPods []*co return false } +func isPodDead(pod *corev1.Pod) bool { + for _, check := range []func(*corev1.Pod) bool{ + isPodDeletedByTaintManager, + } { + if check(pod) { + return true + } + } + return false +} + +func isPodDeletedByTaintManager(pod *corev1.Pod) bool { + if pod.DeletionTimestamp == nil { + return false + } + for _, condition := range pod.Status.Conditions { + if condition.Type == corev1.DisruptionTarget && condition.Reason == "DeletionByTaintManager" && condition.Status == corev1.ConditionTrue { + return true + } + } + return false +} + // imageID returns the ImageID of the primary catalog source container or an empty string if the image ID isn't available yet. // Note: the pod must be running and the container in a ready status to return a valid ImageID. func imageID(pod *corev1.Pod) string { @@ -545,7 +596,7 @@ func imageID(pod *corev1.Pod) string { func (c *GrpcRegistryReconciler) removePods(pods []*corev1.Pod, namespace string) error { for _, p := range pods { if err := c.OpClient.KubernetesInterface().CoreV1().Pods(namespace).Delete(context.TODO(), p.GetName(), *metav1.NewDeleteOptions(1)); err != nil && !apierrors.IsNotFound(err) { - return errors.Wrapf(err, "error deleting pod: %s", p.GetName()) + return pkgerrors.Wrapf(err, "error deleting pod: %s", p.GetName()) } } return nil @@ -623,7 +674,7 @@ func (c *GrpcRegistryReconciler) podFailed(pod *corev1.Pod) (bool, error) { logrus.WithField("UpdatePod", pod.GetName()).Infof("catalog polling result: update pod %s failed to start", pod.GetName()) err := c.removePods([]*corev1.Pod{pod}, pod.GetNamespace()) if err != nil { - return true, errors.Wrapf(err, "error deleting failed catalog polling pod: %s", pod.GetName()) + return true, pkgerrors.Wrapf(err, "error deleting failed catalog polling pod: %s", pod.GetName()) } return true, nil } diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/reconciler/grpc.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/reconciler/grpc.go index e124bb07d3..f13aa17798 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/reconciler/grpc.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/reconciler/grpc.go @@ -2,19 +2,22 @@ package reconciler import ( "context" + "errors" "fmt" + "strings" "time" "github.com/google/go-cmp/cmp" "github.com/operator-framework/api/pkg/operators/v1alpha1" - "github.com/pkg/errors" + pkgerrors "github.com/pkg/errors" "github.com/sirupsen/logrus" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install" controllerclient "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/controller-runtime/client" @@ -262,7 +265,7 @@ func (c *GrpcRegistryReconciler) EnsureRegistryServer(logger *logrus.Entry, cata //TODO: if any of these error out, we should write a status back (possibly set RegistryServiceStatus to nil so they get recreated) sa, err := c.ensureSA(source) if err != nil && !apierrors.IsAlreadyExists(err) { - return errors.Wrapf(err, "error ensuring service account: %s", source.GetName()) + return pkgerrors.Wrapf(err, "error ensuring service account: %s", source.GetName()) } sa, err = c.OpClient.GetServiceAccount(sa.GetNamespace(), sa.GetName()) @@ -285,20 +288,20 @@ func (c *GrpcRegistryReconciler) EnsureRegistryServer(logger *logrus.Entry, cata return err } if err := c.ensurePod(logger, source, sa, overwritePod); err != nil { - return errors.Wrapf(err, "error ensuring pod: %s", pod.GetName()) + return pkgerrors.Wrapf(err, "error ensuring pod: %s", pod.GetName()) } if err := c.ensureUpdatePod(logger, sa, source); err != nil { if _, ok := err.(UpdateNotReadyErr); ok { return err } - return errors.Wrapf(err, "error ensuring updated catalog source pod: %s", pod.GetName()) + return pkgerrors.Wrapf(err, "error ensuring updated catalog source pod: %s", pod.GetName()) } service, err := source.Service() if err != nil { return err } if err := c.ensureService(source, overwrite); err != nil { - return errors.Wrapf(err, "error ensuring service: %s", service.GetName()) + return pkgerrors.Wrapf(err, "error ensuring service: %s", service.GetName()) } if overwritePod { @@ -338,16 +341,41 @@ func isRegistryServiceStatusValid(source *grpcCatalogSourceDecorator) (bool, err } func (c *GrpcRegistryReconciler) ensurePod(logger *logrus.Entry, source grpcCatalogSourceDecorator, serviceAccount *corev1.ServiceAccount, overwrite bool) error { - // currentLivePods refers to the currently live instances of the catalog source - currentLivePods := c.currentPods(logger, source) - if len(currentLivePods) > 0 { + // currentPods refers to the current pod instances of the catalog source + currentPods := c.currentPods(logger, source) + + var forceDeleteErrs []error + // Remove dead pods from the slice without allocating a new slice + // See https://go.dev/wiki/SliceTricks#filtering-without-allocating + tmpSlice := currentPods[:0] + for _, pod := range currentPods { + if !isPodDead(pod) { + logger.WithFields(logrus.Fields{"pod.namespace": source.GetNamespace(), "pod.name": pod.GetName()}).Debug("pod is alive") + tmpSlice = append(tmpSlice, pod) + continue + } + + logger.WithFields(logrus.Fields{"pod.namespace": source.GetNamespace(), "pod.name": pod.GetName()}).Info("force deleting dead pod") + if err := c.OpClient.KubernetesInterface().CoreV1().Pods(source.GetNamespace()).Delete(context.TODO(), pod.GetName(), metav1.DeleteOptions{ + GracePeriodSeconds: ptr.To[int64](0), + }); err != nil && !apierrors.IsNotFound(err) { + forceDeleteErrs = append(forceDeleteErrs, pkgerrors.Wrapf(err, "error deleting old pod: %s", pod.GetName())) + } + } + currentPods = tmpSlice + + if len(forceDeleteErrs) > 0 { + return errors.Join(forceDeleteErrs...) + } + + if len(currentPods) > 0 { if !overwrite { return nil } - for _, p := range currentLivePods { + for _, p := range currentPods { logger.WithFields(logrus.Fields{"pod.namespace": source.GetNamespace(), "pod.name": p.GetName()}).Info("deleting current pod") if err := c.OpClient.KubernetesInterface().CoreV1().Pods(source.GetNamespace()).Delete(context.TODO(), p.GetName(), *metav1.NewDeleteOptions(1)); err != nil && !apierrors.IsNotFound(err) { - return errors.Wrapf(err, "error deleting old pod: %s", p.GetName()) + return pkgerrors.Wrapf(err, "error deleting old pod: %s", p.GetName()) } } } @@ -358,7 +386,7 @@ func (c *GrpcRegistryReconciler) ensurePod(logger *logrus.Entry, source grpcCata logger.WithFields(logrus.Fields{"pod.namespace": source.GetNamespace(), "pod.name": desiredPod.Namespace}).Info("deleting current pod") _, err = c.OpClient.KubernetesInterface().CoreV1().Pods(source.GetNamespace()).Create(context.TODO(), desiredPod, metav1.CreateOptions{}) if err != nil { - return errors.Wrapf(err, "error creating new pod: %s", desiredPod.GetGenerateName()) + return pkgerrors.Wrapf(err, "error creating new pod: %s", desiredPod.GetGenerateName()) } return nil @@ -378,7 +406,7 @@ func (c *GrpcRegistryReconciler) ensureUpdatePod(logger *logrus.Entry, serviceAc logger.Infof("catalog update required at %s", time.Now().String()) pod, err := c.createUpdatePod(source, serviceAccount) if err != nil { - return errors.Wrapf(err, "creating update catalog source pod") + return pkgerrors.Wrapf(err, "creating update catalog source pod") } source.SetLastUpdateTime() return UpdateNotReadyErr{catalogName: source.GetName(), podName: pod.GetName()} @@ -410,7 +438,7 @@ func (c *GrpcRegistryReconciler) ensureUpdatePod(logger *logrus.Entry, serviceAc for _, p := range currentLivePods { logger.WithFields(logrus.Fields{"live-pod.namespace": source.GetNamespace(), "live-pod.name": p.Name}).Info("deleting current live pods") if err := c.OpClient.KubernetesInterface().CoreV1().Pods(source.GetNamespace()).Delete(context.TODO(), p.GetName(), *metav1.NewDeleteOptions(1)); err != nil && !apierrors.IsNotFound(err) { - return errors.Wrapf(errors.Wrapf(err, "error deleting pod: %s", p.GetName()), "detected imageID change: error deleting old catalog source pod") + return pkgerrors.Wrapf(pkgerrors.Wrapf(err, "error deleting pod: %s", p.GetName()), "detected imageID change: error deleting old catalog source pod") } } // done syncing @@ -420,7 +448,7 @@ func (c *GrpcRegistryReconciler) ensureUpdatePod(logger *logrus.Entry, serviceAc // delete update pod right away, since the digest match, to prevent long-lived duplicate catalog pods logger.WithFields(logrus.Fields{"update-pod.namespace": updatePod.Namespace, "update-pod.name": updatePod.Name}).Debug("catalog polling result: no update; removing duplicate update pod") if err := c.OpClient.KubernetesInterface().CoreV1().Pods(source.GetNamespace()).Delete(context.TODO(), updatePod.GetName(), *metav1.NewDeleteOptions(1)); err != nil && !apierrors.IsNotFound(err) { - return errors.Wrapf(errors.Wrapf(err, "error deleting pod: %s", updatePod.GetName()), "duplicate catalog polling pod") + return pkgerrors.Wrapf(pkgerrors.Wrapf(err, "error deleting pod: %s", updatePod.GetName()), "duplicate catalog polling pod") } } @@ -523,6 +551,29 @@ func imageChanged(logger *logrus.Entry, updatePod *corev1.Pod, servingPods []*co return false } +func isPodDead(pod *corev1.Pod) bool { + for _, check := range []func(*corev1.Pod) bool{ + isPodDeletedByTaintManager, + } { + if check(pod) { + return true + } + } + return false +} + +func isPodDeletedByTaintManager(pod *corev1.Pod) bool { + if pod.DeletionTimestamp == nil { + return false + } + for _, condition := range pod.Status.Conditions { + if condition.Type == corev1.DisruptionTarget && condition.Reason == "DeletionByTaintManager" && condition.Status == corev1.ConditionTrue { + return true + } + } + return false +} + // imageID returns the ImageID of the primary catalog source container or an empty string if the image ID isn't available yet. // Note: the pod must be running and the container in a ready status to return a valid ImageID. func imageID(pod *corev1.Pod) string { @@ -545,7 +596,7 @@ func imageID(pod *corev1.Pod) string { func (c *GrpcRegistryReconciler) removePods(pods []*corev1.Pod, namespace string) error { for _, p := range pods { if err := c.OpClient.KubernetesInterface().CoreV1().Pods(namespace).Delete(context.TODO(), p.GetName(), *metav1.NewDeleteOptions(1)); err != nil && !apierrors.IsNotFound(err) { - return errors.Wrapf(err, "error deleting pod: %s", p.GetName()) + return pkgerrors.Wrapf(err, "error deleting pod: %s", p.GetName()) } } return nil @@ -623,7 +674,7 @@ func (c *GrpcRegistryReconciler) podFailed(pod *corev1.Pod) (bool, error) { logrus.WithField("UpdatePod", pod.GetName()).Infof("catalog polling result: update pod %s failed to start", pod.GetName()) err := c.removePods([]*corev1.Pod{pod}, pod.GetNamespace()) if err != nil { - return true, errors.Wrapf(err, "error deleting failed catalog polling pod: %s", pod.GetName()) + return true, pkgerrors.Wrapf(err, "error deleting failed catalog polling pod: %s", pod.GetName()) } return true, nil } diff --git a/vendor/k8s.io/utils/integer/integer.go b/vendor/k8s.io/utils/integer/integer.go index e4e740cad4..e0811e8344 100644 --- a/vendor/k8s.io/utils/integer/integer.go +++ b/vendor/k8s.io/utils/integer/integer.go @@ -16,6 +16,8 @@ limitations under the License. package integer +import "math" + // IntMax returns the maximum of the params func IntMax(a, b int) int { if b > a { @@ -65,9 +67,7 @@ func Int64Min(a, b int64) int64 { } // RoundToInt32 rounds floats into integer numbers. +// Deprecated: use math.Round() and a cast directly. func RoundToInt32(a float64) int32 { - if a < 0 { - return int32(a - 0.5) - } - return int32(a + 0.5) + return int32(math.Round(a)) } diff --git a/vendor/k8s.io/utils/pointer/pointer.go b/vendor/k8s.io/utils/pointer/pointer.go index b8103223ad..b673a64257 100644 --- a/vendor/k8s.io/utils/pointer/pointer.go +++ b/vendor/k8s.io/utils/pointer/pointer.go @@ -14,12 +14,15 @@ See the License for the specific language governing permissions and limitations under the License. */ +// Deprecated: Use functions in k8s.io/utils/ptr instead: ptr.To to obtain +// a pointer, ptr.Deref to dereference a pointer, ptr.Equal to compare +// dereferenced pointers. package pointer import ( - "fmt" - "reflect" "time" + + "k8s.io/utils/ptr" ) // AllPtrFieldsNil tests whether all pointer fields in a struct are nil. This is useful when, @@ -28,383 +31,219 @@ import ( // // This function is only valid for structs and pointers to structs. Any other // type will cause a panic. Passing a typed nil pointer will return true. -func AllPtrFieldsNil(obj interface{}) bool { - v := reflect.ValueOf(obj) - if !v.IsValid() { - panic(fmt.Sprintf("reflect.ValueOf() produced a non-valid Value for %#v", obj)) - } - if v.Kind() == reflect.Ptr { - if v.IsNil() { - return true - } - v = v.Elem() - } - for i := 0; i < v.NumField(); i++ { - if v.Field(i).Kind() == reflect.Ptr && !v.Field(i).IsNil() { - return false - } - } - return true -} - -// Int returns a pointer to an int -func Int(i int) *int { - return &i -} +// +// Deprecated: Use ptr.AllPtrFieldsNil instead. +var AllPtrFieldsNil = ptr.AllPtrFieldsNil + +// Int returns a pointer to an int. +var Int = ptr.To[int] // IntPtr is a function variable referring to Int. // -// Deprecated: Use Int instead. +// Deprecated: Use ptr.To instead. var IntPtr = Int // for back-compat // IntDeref dereferences the int ptr and returns it if not nil, or else // returns def. -func IntDeref(ptr *int, def int) int { - if ptr != nil { - return *ptr - } - return def -} +var IntDeref = ptr.Deref[int] // IntPtrDerefOr is a function variable referring to IntDeref. // -// Deprecated: Use IntDeref instead. +// Deprecated: Use ptr.Deref instead. var IntPtrDerefOr = IntDeref // for back-compat // Int32 returns a pointer to an int32. -func Int32(i int32) *int32 { - return &i -} +var Int32 = ptr.To[int32] // Int32Ptr is a function variable referring to Int32. // -// Deprecated: Use Int32 instead. +// Deprecated: Use ptr.To instead. var Int32Ptr = Int32 // for back-compat // Int32Deref dereferences the int32 ptr and returns it if not nil, or else // returns def. -func Int32Deref(ptr *int32, def int32) int32 { - if ptr != nil { - return *ptr - } - return def -} +var Int32Deref = ptr.Deref[int32] // Int32PtrDerefOr is a function variable referring to Int32Deref. // -// Deprecated: Use Int32Deref instead. +// Deprecated: Use ptr.Deref instead. var Int32PtrDerefOr = Int32Deref // for back-compat // Int32Equal returns true if both arguments are nil or both arguments // dereference to the same value. -func Int32Equal(a, b *int32) bool { - if (a == nil) != (b == nil) { - return false - } - if a == nil { - return true - } - return *a == *b -} +var Int32Equal = ptr.Equal[int32] // Uint returns a pointer to an uint -func Uint(i uint) *uint { - return &i -} +var Uint = ptr.To[uint] // UintPtr is a function variable referring to Uint. // -// Deprecated: Use Uint instead. +// Deprecated: Use ptr.To instead. var UintPtr = Uint // for back-compat // UintDeref dereferences the uint ptr and returns it if not nil, or else // returns def. -func UintDeref(ptr *uint, def uint) uint { - if ptr != nil { - return *ptr - } - return def -} +var UintDeref = ptr.Deref[uint] // UintPtrDerefOr is a function variable referring to UintDeref. // -// Deprecated: Use UintDeref instead. +// Deprecated: Use ptr.Deref instead. var UintPtrDerefOr = UintDeref // for back-compat // Uint32 returns a pointer to an uint32. -func Uint32(i uint32) *uint32 { - return &i -} +var Uint32 = ptr.To[uint32] // Uint32Ptr is a function variable referring to Uint32. // -// Deprecated: Use Uint32 instead. +// Deprecated: Use ptr.To instead. var Uint32Ptr = Uint32 // for back-compat // Uint32Deref dereferences the uint32 ptr and returns it if not nil, or else // returns def. -func Uint32Deref(ptr *uint32, def uint32) uint32 { - if ptr != nil { - return *ptr - } - return def -} +var Uint32Deref = ptr.Deref[uint32] // Uint32PtrDerefOr is a function variable referring to Uint32Deref. // -// Deprecated: Use Uint32Deref instead. +// Deprecated: Use ptr.Deref instead. var Uint32PtrDerefOr = Uint32Deref // for back-compat // Uint32Equal returns true if both arguments are nil or both arguments // dereference to the same value. -func Uint32Equal(a, b *uint32) bool { - if (a == nil) != (b == nil) { - return false - } - if a == nil { - return true - } - return *a == *b -} +var Uint32Equal = ptr.Equal[uint32] // Int64 returns a pointer to an int64. -func Int64(i int64) *int64 { - return &i -} +var Int64 = ptr.To[int64] // Int64Ptr is a function variable referring to Int64. // -// Deprecated: Use Int64 instead. +// Deprecated: Use ptr.To instead. var Int64Ptr = Int64 // for back-compat // Int64Deref dereferences the int64 ptr and returns it if not nil, or else // returns def. -func Int64Deref(ptr *int64, def int64) int64 { - if ptr != nil { - return *ptr - } - return def -} +var Int64Deref = ptr.Deref[int64] // Int64PtrDerefOr is a function variable referring to Int64Deref. // -// Deprecated: Use Int64Deref instead. +// Deprecated: Use ptr.Deref instead. var Int64PtrDerefOr = Int64Deref // for back-compat // Int64Equal returns true if both arguments are nil or both arguments // dereference to the same value. -func Int64Equal(a, b *int64) bool { - if (a == nil) != (b == nil) { - return false - } - if a == nil { - return true - } - return *a == *b -} +var Int64Equal = ptr.Equal[int64] // Uint64 returns a pointer to an uint64. -func Uint64(i uint64) *uint64 { - return &i -} +var Uint64 = ptr.To[uint64] // Uint64Ptr is a function variable referring to Uint64. // -// Deprecated: Use Uint64 instead. +// Deprecated: Use ptr.To instead. var Uint64Ptr = Uint64 // for back-compat // Uint64Deref dereferences the uint64 ptr and returns it if not nil, or else // returns def. -func Uint64Deref(ptr *uint64, def uint64) uint64 { - if ptr != nil { - return *ptr - } - return def -} +var Uint64Deref = ptr.Deref[uint64] // Uint64PtrDerefOr is a function variable referring to Uint64Deref. // -// Deprecated: Use Uint64Deref instead. +// Deprecated: Use ptr.Deref instead. var Uint64PtrDerefOr = Uint64Deref // for back-compat // Uint64Equal returns true if both arguments are nil or both arguments // dereference to the same value. -func Uint64Equal(a, b *uint64) bool { - if (a == nil) != (b == nil) { - return false - } - if a == nil { - return true - } - return *a == *b -} +var Uint64Equal = ptr.Equal[uint64] // Bool returns a pointer to a bool. -func Bool(b bool) *bool { - return &b -} +var Bool = ptr.To[bool] // BoolPtr is a function variable referring to Bool. // -// Deprecated: Use Bool instead. +// Deprecated: Use ptr.To instead. var BoolPtr = Bool // for back-compat // BoolDeref dereferences the bool ptr and returns it if not nil, or else // returns def. -func BoolDeref(ptr *bool, def bool) bool { - if ptr != nil { - return *ptr - } - return def -} +var BoolDeref = ptr.Deref[bool] // BoolPtrDerefOr is a function variable referring to BoolDeref. // -// Deprecated: Use BoolDeref instead. +// Deprecated: Use ptr.Deref instead. var BoolPtrDerefOr = BoolDeref // for back-compat // BoolEqual returns true if both arguments are nil or both arguments // dereference to the same value. -func BoolEqual(a, b *bool) bool { - if (a == nil) != (b == nil) { - return false - } - if a == nil { - return true - } - return *a == *b -} +var BoolEqual = ptr.Equal[bool] // String returns a pointer to a string. -func String(s string) *string { - return &s -} +var String = ptr.To[string] // StringPtr is a function variable referring to String. // -// Deprecated: Use String instead. +// Deprecated: Use ptr.To instead. var StringPtr = String // for back-compat // StringDeref dereferences the string ptr and returns it if not nil, or else // returns def. -func StringDeref(ptr *string, def string) string { - if ptr != nil { - return *ptr - } - return def -} +var StringDeref = ptr.Deref[string] // StringPtrDerefOr is a function variable referring to StringDeref. // -// Deprecated: Use StringDeref instead. +// Deprecated: Use ptr.Deref instead. var StringPtrDerefOr = StringDeref // for back-compat // StringEqual returns true if both arguments are nil or both arguments // dereference to the same value. -func StringEqual(a, b *string) bool { - if (a == nil) != (b == nil) { - return false - } - if a == nil { - return true - } - return *a == *b -} +var StringEqual = ptr.Equal[string] // Float32 returns a pointer to a float32. -func Float32(i float32) *float32 { - return &i -} +var Float32 = ptr.To[float32] // Float32Ptr is a function variable referring to Float32. // -// Deprecated: Use Float32 instead. +// Deprecated: Use ptr.To instead. var Float32Ptr = Float32 // Float32Deref dereferences the float32 ptr and returns it if not nil, or else // returns def. -func Float32Deref(ptr *float32, def float32) float32 { - if ptr != nil { - return *ptr - } - return def -} +var Float32Deref = ptr.Deref[float32] // Float32PtrDerefOr is a function variable referring to Float32Deref. // -// Deprecated: Use Float32Deref instead. +// Deprecated: Use ptr.Deref instead. var Float32PtrDerefOr = Float32Deref // for back-compat // Float32Equal returns true if both arguments are nil or both arguments // dereference to the same value. -func Float32Equal(a, b *float32) bool { - if (a == nil) != (b == nil) { - return false - } - if a == nil { - return true - } - return *a == *b -} +var Float32Equal = ptr.Equal[float32] // Float64 returns a pointer to a float64. -func Float64(i float64) *float64 { - return &i -} +var Float64 = ptr.To[float64] // Float64Ptr is a function variable referring to Float64. // -// Deprecated: Use Float64 instead. +// Deprecated: Use ptr.To instead. var Float64Ptr = Float64 // Float64Deref dereferences the float64 ptr and returns it if not nil, or else // returns def. -func Float64Deref(ptr *float64, def float64) float64 { - if ptr != nil { - return *ptr - } - return def -} +var Float64Deref = ptr.Deref[float64] // Float64PtrDerefOr is a function variable referring to Float64Deref. // -// Deprecated: Use Float64Deref instead. +// Deprecated: Use ptr.Deref instead. var Float64PtrDerefOr = Float64Deref // for back-compat // Float64Equal returns true if both arguments are nil or both arguments // dereference to the same value. -func Float64Equal(a, b *float64) bool { - if (a == nil) != (b == nil) { - return false - } - if a == nil { - return true - } - return *a == *b -} +var Float64Equal = ptr.Equal[float64] // Duration returns a pointer to a time.Duration. -func Duration(d time.Duration) *time.Duration { - return &d -} +var Duration = ptr.To[time.Duration] // DurationDeref dereferences the time.Duration ptr and returns it if not nil, or else // returns def. -func DurationDeref(ptr *time.Duration, def time.Duration) time.Duration { - if ptr != nil { - return *ptr - } - return def -} +var DurationDeref = ptr.Deref[time.Duration] // DurationEqual returns true if both arguments are nil or both arguments // dereference to the same value. -func DurationEqual(a, b *time.Duration) bool { - if (a == nil) != (b == nil) { - return false - } - if a == nil { - return true - } - return *a == *b -} +var DurationEqual = ptr.Equal[time.Duration] diff --git a/vendor/k8s.io/utils/ptr/OWNERS b/vendor/k8s.io/utils/ptr/OWNERS new file mode 100644 index 0000000000..0d6392752a --- /dev/null +++ b/vendor/k8s.io/utils/ptr/OWNERS @@ -0,0 +1,10 @@ +# See the OWNERS docs at https://go.k8s.io/owners + +approvers: +- apelisse +- stewart-yu +- thockin +reviewers: +- apelisse +- stewart-yu +- thockin diff --git a/vendor/k8s.io/utils/ptr/README.md b/vendor/k8s.io/utils/ptr/README.md new file mode 100644 index 0000000000..2ca8073dc7 --- /dev/null +++ b/vendor/k8s.io/utils/ptr/README.md @@ -0,0 +1,3 @@ +# Pointer + +This package provides some functions for pointer-based operations. diff --git a/vendor/k8s.io/utils/ptr/ptr.go b/vendor/k8s.io/utils/ptr/ptr.go new file mode 100644 index 0000000000..659ed3b9e2 --- /dev/null +++ b/vendor/k8s.io/utils/ptr/ptr.go @@ -0,0 +1,73 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package ptr + +import ( + "fmt" + "reflect" +) + +// AllPtrFieldsNil tests whether all pointer fields in a struct are nil. This is useful when, +// for example, an API struct is handled by plugins which need to distinguish +// "no plugin accepted this spec" from "this spec is empty". +// +// This function is only valid for structs and pointers to structs. Any other +// type will cause a panic. Passing a typed nil pointer will return true. +func AllPtrFieldsNil(obj interface{}) bool { + v := reflect.ValueOf(obj) + if !v.IsValid() { + panic(fmt.Sprintf("reflect.ValueOf() produced a non-valid Value for %#v", obj)) + } + if v.Kind() == reflect.Ptr { + if v.IsNil() { + return true + } + v = v.Elem() + } + for i := 0; i < v.NumField(); i++ { + if v.Field(i).Kind() == reflect.Ptr && !v.Field(i).IsNil() { + return false + } + } + return true +} + +// To returns a pointer to the given value. +func To[T any](v T) *T { + return &v +} + +// Deref dereferences ptr and returns the value it points to if no nil, or else +// returns def. +func Deref[T any](ptr *T, def T) T { + if ptr != nil { + return *ptr + } + return def +} + +// Equal returns true if both arguments are nil or both arguments +// dereference to the same value. +func Equal[T comparable](a, b *T) bool { + if (a == nil) != (b == nil) { + return false + } + if a == nil { + return true + } + return *a == *b +} diff --git a/vendor/k8s.io/utils/trace/trace.go b/vendor/k8s.io/utils/trace/trace.go index a0b07a6d78..559aebb59a 100644 --- a/vendor/k8s.io/utils/trace/trace.go +++ b/vendor/k8s.io/utils/trace/trace.go @@ -65,6 +65,11 @@ func durationToMilliseconds(timeDuration time.Duration) int64 { } type traceItem interface { + // rLock must be called before invoking time or writeItem. + rLock() + // rUnlock must be called after processing the item is complete. + rUnlock() + // time returns when the trace was recorded as completed. time() time.Time // writeItem outputs the traceItem to the buffer. If stepThreshold is non-nil, only output the @@ -79,6 +84,10 @@ type traceStep struct { fields []Field } +// rLock doesn't need to do anything because traceStep instances are immutable. +func (s traceStep) rLock() {} +func (s traceStep) rUnlock() {} + func (s traceStep) time() time.Time { return s.stepTime } @@ -106,6 +115,14 @@ type Trace struct { traceItems []traceItem } +func (t *Trace) rLock() { + t.lock.RLock() +} + +func (t *Trace) rUnlock() { + t.lock.RUnlock() +} + func (t *Trace) time() time.Time { if t.endTime != nil { return *t.endTime @@ -175,7 +192,7 @@ func (t *Trace) Log() { t.endTime = &endTime t.lock.Unlock() // an explicit logging request should dump all the steps out at the higher level - if t.parentTrace == nil { // We don't start logging until Log or LogIfLong is called on the root trace + if t.parentTrace == nil && klogV(2) { // We don't start logging until Log or LogIfLong is called on the root trace t.logTrace() } } @@ -231,8 +248,10 @@ func (t *Trace) logTrace() { func (t *Trace) writeTraceSteps(b *bytes.Buffer, formatter string, stepThreshold *time.Duration) { lastStepTime := t.startTime for _, stepOrTrace := range t.traceItems { + stepOrTrace.rLock() stepOrTrace.writeItem(b, formatter, lastStepTime, stepThreshold) lastStepTime = stepOrTrace.time() + stepOrTrace.rUnlock() } } diff --git a/vendor/modules.txt b/vendor/modules.txt index b7f322c1a3..6ddc266deb 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -2161,7 +2161,7 @@ k8s.io/kubectl/pkg/util/slice k8s.io/kubectl/pkg/util/templates k8s.io/kubectl/pkg/util/term k8s.io/kubectl/pkg/validation -# k8s.io/utils v0.0.0-20230220204549-a5ecb0141aa5 +# k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0 ## explicit; go 1.18 k8s.io/utils/buffer k8s.io/utils/clock @@ -2174,6 +2174,7 @@ k8s.io/utils/lru k8s.io/utils/net k8s.io/utils/path k8s.io/utils/pointer +k8s.io/utils/ptr k8s.io/utils/strings/slices k8s.io/utils/trace # oras.land/oras-go v1.2.4