From b76d1c647deae8407d3e8d99f579757c62f546d2 Mon Sep 17 00:00:00 2001 From: ajanikow <12255597+ajanikow@users.noreply.github.com> Date: Fri, 5 Jun 2020 13:46:26 +0000 Subject: [PATCH 1/3] Improve reconcilation time --- main.go | 5 + pkg/deployment/cleanup.go | 18 ++- pkg/deployment/context_impl.go | 40 +---- pkg/deployment/deployment.go | 42 ++--- pkg/deployment/deployment_finalizers.go | 10 +- pkg/deployment/deployment_inspector.go | 84 ++++++---- pkg/deployment/deployment_run_test.go | 5 +- pkg/deployment/images.go | 11 +- pkg/deployment/pod/affinity.go | 5 +- pkg/deployment/pod/builder.go | 3 +- pkg/deployment/pod/encryption.go | 26 +++- pkg/deployment/pod/jwt.go | 61 ++++++++ pkg/deployment/pod/sni.go | 11 +- pkg/deployment/pod/upgrade.go | 3 +- pkg/deployment/reconcile/action_context.go | 20 ++- .../reconcile/action_tls_sni_update.go | 2 +- pkg/deployment/reconcile/context.go | 9 +- pkg/deployment/reconcile/helper_tls_sni.go | 11 +- pkg/deployment/reconcile/plan_builder.go | 86 +++++++---- .../reconcile/plan_builder_context.go | 7 +- .../reconcile/plan_builder_encryption.go | 45 ++++-- .../reconcile/plan_builder_restore.go | 12 +- .../reconcile/plan_builder_rotate_upgrade.go | 43 ++++-- .../reconcile/plan_builder_scale.go | 9 +- .../reconcile/plan_builder_storage.go | 18 ++- pkg/deployment/reconcile/plan_builder_test.go | 98 ++++++------ pkg/deployment/reconcile/plan_builder_tls.go | 43 ++++-- .../reconcile/plan_builder_tls_sni.go | 11 +- pkg/deployment/reconcile/plan_executor.go | 10 +- pkg/deployment/resources/annotations.go | 88 ++++++----- pkg/deployment/resources/context.go | 4 +- .../resources/inspector/inspector.go | 86 +++++++++++ pkg/deployment/resources/inspector/pods.go | 124 +++++++++++++++ pkg/deployment/resources/inspector/pvcs.go | 124 +++++++++++++++ pkg/deployment/resources/inspector/secrets.go | 107 +++++++++++++ .../resources/inspector/services.go | 107 +++++++++++++ pkg/deployment/resources/license.go | 16 +- pkg/deployment/resources/pod_cleanup.go | 46 +++--- pkg/deployment/resources/pod_creator.go | 31 ++-- .../resources/pod_creator_arangod.go | 21 ++- pkg/deployment/resources/pod_creator_sync.go | 11 +- pkg/deployment/resources/pod_inspector.go | 82 +++++----- pkg/deployment/resources/pvc_inspector.go | 33 ++-- pkg/deployment/resources/pvcs.go | 28 ++-- pkg/deployment/resources/secret_hashes.go | 44 ++++-- pkg/deployment/resources/secrets.go | 144 +++++++++++------- pkg/deployment/resources/services.go | 23 +-- pkg/operator/errors.go | 15 +- pkg/util/errors/errors.go | 31 ++++ pkg/util/k8sutil/interfaces/pod_creator.go | 60 ++++++++ pkg/util/k8sutil/pods.go | 42 +---- pkg/util/k8sutil/pvc.go | 2 +- pkg/util/k8sutil/secrets.go | 85 +++++------ pkg/util/k8sutil/services.go | 12 +- pkg/util/k8sutil/util.go | 4 +- pkg/util/k8sutil/util_test.go | 6 +- 56 files changed, 1486 insertions(+), 638 deletions(-) create mode 100644 pkg/deployment/pod/jwt.go create mode 100644 pkg/deployment/resources/inspector/inspector.go create mode 100644 pkg/deployment/resources/inspector/pods.go create mode 100644 pkg/deployment/resources/inspector/pvcs.go create mode 100644 pkg/deployment/resources/inspector/secrets.go create mode 100644 pkg/deployment/resources/inspector/services.go create mode 100644 pkg/util/k8sutil/interfaces/pod_creator.go diff --git a/main.go b/main.go index 26d4f75ab..b06bcdb5e 100644 --- a/main.go +++ b/main.go @@ -31,6 +31,8 @@ import ( "strings" "time" + "github.com/rs/zerolog/log" + deploymentApi "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" "github.com/arangodb/kube-arangodb/pkg/util" @@ -146,6 +148,9 @@ func cmdUsage(cmd *cobra.Command, args []string) { // Run the operator func cmdMainRun(cmd *cobra.Command, args []string) { + // Set global logger + log.Logger = logging.NewRootLogger() + // Get environment namespace := os.Getenv(constants.EnvOperatorPodNamespace) name := os.Getenv(constants.EnvOperatorPodName) diff --git a/pkg/deployment/cleanup.go b/pkg/deployment/cleanup.go index a6bb2fe77..abe0939f4 100644 --- a/pkg/deployment/cleanup.go +++ b/pkg/deployment/cleanup.go @@ -23,23 +23,25 @@ package deployment import ( + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" + core "k8s.io/api/core/v1" ) // removePodFinalizers removes all finalizers from all pods owned by us. -func (d *Deployment) removePodFinalizers() error { +func (d *Deployment) removePodFinalizers(cachedStatus inspector.Inspector) error { log := d.deps.Log kubecli := d.GetKubeCli() - pods, err := d.GetOwnedPods() - if err != nil { - return maskAny(err) - } - for _, p := range pods { - ignoreNotFound := true - if err := k8sutil.RemovePodFinalizers(log, kubecli, &p, p.GetFinalizers(), ignoreNotFound); err != nil { + + if err := cachedStatus.IteratePods(func(pod *core.Pod) error { + if err := k8sutil.RemovePodFinalizers(log, kubecli, pod, pod.GetFinalizers(), true); err != nil { log.Warn().Err(err).Msg("Failed to remove pod finalizers") } + return nil + }, inspector.FilterPodsByLabels(k8sutil.LabelsForDeployment(d.GetName(), ""))); err != nil { + return err } + return nil } diff --git a/pkg/deployment/context_impl.go b/pkg/deployment/context_impl.go index cd856aab4..2ca3202c6 100644 --- a/pkg/deployment/context_impl.go +++ b/pkg/deployment/context_impl.go @@ -28,6 +28,8 @@ import ( "net" "strconv" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + "k8s.io/apimachinery/pkg/api/errors" "github.com/arangodb/arangosync-client/client" @@ -275,7 +277,7 @@ func (d *Deployment) DeletePod(podName string) error { // CleanupPod deletes a given pod with force and explicit UID. // If the pod does not exist, the error is ignored. -func (d *Deployment) CleanupPod(p v1.Pod) error { +func (d *Deployment) CleanupPod(p *v1.Pod) error { log := d.deps.Log podName := p.GetName() ns := p.GetNamespace() @@ -344,24 +346,6 @@ func (d *Deployment) GetPv(pvName string) (*v1.PersistentVolume, error) { return nil, maskAny(err) } -// GetOwnedPods returns a list of all pods owned by the deployment. -func (d *Deployment) GetOwnedPods() ([]v1.Pod, error) { - // Get all current pods - log := d.deps.Log - pods, err := d.deps.KubeCli.CoreV1().Pods(d.apiObject.GetNamespace()).List(k8sutil.DeploymentListOpt(d.apiObject.GetName())) - if err != nil { - log.Debug().Err(err).Msg("Failed to list pods") - return nil, maskAny(err) - } - myPods := make([]v1.Pod, 0, len(pods.Items)) - for _, p := range pods.Items { - if d.isOwnerOf(&p) { - myPods = append(myPods, p) - } - } - return myPods, nil -} - // GetOwnedPVCs returns a list of all PVCs owned by the deployment. func (d *Deployment) GetOwnedPVCs() ([]v1.PersistentVolumeClaim, error) { // Get all current PVCs @@ -414,20 +398,6 @@ func (d *Deployment) DeleteTLSKeyfile(group api.ServerGroup, member api.MemberSt return nil } -// GetTLSCA returns the TLS CA certificate in the secret with given name. -// Returns: publicKey, privateKey, ownerByDeployment, error -func (d *Deployment) GetTLSCA(secretName string) (string, string, bool, error) { - ns := d.apiObject.GetNamespace() - secrets := d.deps.KubeCli.CoreV1().Secrets(ns) - owner := d.apiObject.AsOwner() - cert, priv, isOwned, err := k8sutil.GetCASecret(secrets, secretName, &owner) - if err != nil { - return "", "", false, maskAny(err) - } - return cert, priv, isOwned, nil - -} - // DeleteSecret removes the Secret with given name. // If the secret does not exist, the error is ignored. func (d *Deployment) DeleteSecret(secretName string) error { @@ -470,8 +440,8 @@ func (d *Deployment) GetAgencyData(ctx context.Context, i interface{}, keyParts return err } -func (d *Deployment) RenderPodForMember(spec api.DeploymentSpec, status api.DeploymentStatus, memberID string, imageInfo api.ImageInfo) (*v1.Pod, error) { - return d.resources.RenderPodForMember(spec, status, memberID, imageInfo) +func (d *Deployment) RenderPodForMember(cachedStatus inspector.Inspector, spec api.DeploymentSpec, status api.DeploymentStatus, memberID string, imageInfo api.ImageInfo) (*v1.Pod, error) { + return d.resources.RenderPodForMember(cachedStatus, spec, status, memberID, imageInfo) } func (d *Deployment) SelectImage(spec api.DeploymentSpec, status api.DeploymentStatus) (api.ImageInfo, bool) { diff --git a/pkg/deployment/deployment.go b/pkg/deployment/deployment.go index 93879f105..b25d56611 100644 --- a/pkg/deployment/deployment.go +++ b/pkg/deployment/deployment.go @@ -29,6 +29,8 @@ import ( "sync/atomic" "time" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + "github.com/arangodb/kube-arangodb/pkg/util/arangod" "github.com/arangodb/arangosync-client/client" @@ -84,7 +86,7 @@ type deploymentEvent struct { const ( deploymentEventQueueSize = 256 minInspectionInterval = 250 * util.Interval(time.Millisecond) // Ensure we inspect the generated resources no less than with this interval - maxInspectionInterval = 30 * util.Interval(time.Second) // Ensure we inspect the generated resources no less than with this interval + maxInspectionInterval = 10 * util.Interval(time.Second) // Ensure we inspect the generated resources no less than with this interval ) // Deployment is the in process state of an ArangoDeployment. @@ -151,10 +153,10 @@ func New(config Config, deps Dependencies, apiObject *api.ArangoDeployment) (*De go d.resources.RunDeploymentHealthLoop(d.stopCh) go d.resources.RunDeploymentShardSyncLoop(d.stopCh) } - if config.AllowChaos { - d.chaosMonkey = chaos.NewMonkey(deps.Log, d) - go d.chaosMonkey.Run(d.stopCh) - } + //if config.AllowChaos { + // d.chaosMonkey = chaos.NewMonkey(deps.Log, d) + // go d.chaosMonkey.Run(d.stopCh) + //} return d, nil } @@ -199,16 +201,6 @@ func (d *Deployment) run() { log := d.deps.Log if d.GetPhase() == api.DeploymentPhaseNone { - // Create secrets - if err := d.resources.EnsureSecrets(); err != nil { - d.CreateEvent(k8sutil.NewErrorEvent("Failed to create secrets", err, d.GetAPIObject())) - } - - // Create services - if err := d.resources.EnsureServices(); err != nil { - d.CreateEvent(k8sutil.NewErrorEvent("Failed to create services", err, d.GetAPIObject())) - } - // Create service monitor if d.haveServiceMonitorCRD { if err := d.resources.EnsureServiceMonitor(); err != nil { @@ -221,16 +213,6 @@ func (d *Deployment) run() { d.CreateEvent(k8sutil.NewErrorEvent("Failed to create initial members", err, d.GetAPIObject())) } - // Create PVCs - if err := d.resources.EnsurePVCs(); err != nil { - d.CreateEvent(k8sutil.NewErrorEvent("Failed to create persistent volume claims", err, d.GetAPIObject())) - } - - // Create pods - if err := d.resources.EnsurePods(); err != nil { - d.CreateEvent(k8sutil.NewErrorEvent("Failed to create pods", err, d.GetAPIObject())) - } - // Create Pod Disruption Budgets if err := d.resources.EnsurePDBs(); err != nil { d.CreateEvent(k8sutil.NewErrorEvent("Failed to create pdbs", err, d.GetAPIObject())) @@ -244,19 +226,19 @@ func (d *Deployment) run() { log.Info().Msg("start running...") } - if err := d.resources.EnsureAnnotations(); err != nil { - log.Warn().Err(err).Msg("unable to update annotations") - } - d.lookForServiceMonitorCRD() inspectionInterval := maxInspectionInterval for { select { case <-d.stopCh: + cachedStatus, err := inspector.NewInspector(d.GetKubeCli(), d.GetNamespace()) + if err != nil { + log.Error().Err(err).Msg("Unable to get resources") + } // Remove finalizers from created resources log.Info().Msg("Deployment removed, removing finalizers to prevent orphaned resources") - if err := d.removePodFinalizers(); err != nil { + if err := d.removePodFinalizers(cachedStatus); err != nil { log.Warn().Err(err).Msg("Failed to remove Pod finalizers") } if err := d.removePVCFinalizers(); err != nil { diff --git a/pkg/deployment/deployment_finalizers.go b/pkg/deployment/deployment_finalizers.go index 52785fc1e..cae2d6c14 100644 --- a/pkg/deployment/deployment_finalizers.go +++ b/pkg/deployment/deployment_finalizers.go @@ -25,6 +25,8 @@ package deployment import ( "context" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + "github.com/rs/zerolog" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -47,7 +49,7 @@ func ensureFinalizers(depl *api.ArangoDeployment) { } // runDeploymentFinalizers goes through the list of ArangoDeployoment finalizers to see if they can be removed. -func (d *Deployment) runDeploymentFinalizers(ctx context.Context) error { +func (d *Deployment) runDeploymentFinalizers(ctx context.Context, cachedStatus inspector.Inspector) error { log := d.deps.Log var removalList []string @@ -60,7 +62,7 @@ func (d *Deployment) runDeploymentFinalizers(ctx context.Context) error { switch f { case constants.FinalizerDeplRemoveChildFinalizers: log.Debug().Msg("Inspecting 'remove child finalizers' finalizer") - if err := d.inspectRemoveChildFinalizers(ctx, log, updated); err == nil { + if err := d.inspectRemoveChildFinalizers(ctx, log, updated, cachedStatus); err == nil { removalList = append(removalList, f) } else { log.Debug().Err(err).Str("finalizer", f).Msg("Cannot remove finalizer yet") @@ -79,8 +81,8 @@ func (d *Deployment) runDeploymentFinalizers(ctx context.Context) error { // inspectRemoveChildFinalizers checks the finalizer condition for remove-child-finalizers. // It returns nil if the finalizer can be removed. -func (d *Deployment) inspectRemoveChildFinalizers(ctx context.Context, log zerolog.Logger, depl *api.ArangoDeployment) error { - if err := d.removePodFinalizers(); err != nil { +func (d *Deployment) inspectRemoveChildFinalizers(ctx context.Context, log zerolog.Logger, depl *api.ArangoDeployment, cachedStatus inspector.Inspector) error { + if err := d.removePodFinalizers(cachedStatus); err != nil { return maskAny(err) } if err := d.removePVCFinalizers(); err != nil { diff --git a/pkg/deployment/deployment_inspector.go b/pkg/deployment/deployment_inspector.go index 6f6f3c10c..c97c9c4b6 100644 --- a/pkg/deployment/deployment_inspector.go +++ b/pkg/deployment/deployment_inspector.go @@ -26,6 +26,10 @@ import ( "context" "time" + operatorErrors "github.com/arangodb/kube-arangodb/pkg/util/errors" + + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + "github.com/pkg/errors" "github.com/arangodb/kube-arangodb/pkg/apis/deployment" @@ -51,6 +55,9 @@ var ( func (d *Deployment) inspectDeployment(lastInterval util.Interval) util.Interval { log := d.deps.Log start := time.Now() + defer func() { + d.deps.Log.Info().Msgf("Inspect loop took %s", time.Since(start)) + }() nextInterval := lastInterval hasError := false @@ -58,6 +65,13 @@ func (d *Deployment) inspectDeployment(lastInterval util.Interval) util.Interval deploymentName := d.apiObject.GetName() defer metrics.SetDuration(inspectDeploymentDurationGauges.WithLabelValues(deploymentName), start) + cachedStatus, err := inspector.NewInspector(d.GetKubeCli(), d.GetNamespace()) + if err != nil { + log.Error().Err(err).Msg("Unable to get resources") + return minInspectionInterval // Retry ASAP + } + + // Check deployment still exists updated, err := d.deps.DatabaseCRCli.DatabaseV1().ArangoDeployments(d.apiObject.GetNamespace()).Get(deploymentName, metav1.GetOptions{}) if k8sutil.IsNotFound(err) { @@ -67,7 +81,7 @@ func (d *Deployment) inspectDeployment(lastInterval util.Interval) util.Interval return nextInterval } else if updated != nil && updated.GetDeletionTimestamp() != nil { // Deployment is marked for deletion - if err := d.runDeploymentFinalizers(ctx); err != nil { + if err := d.runDeploymentFinalizers(ctx, cachedStatus); err != nil { hasError = true d.CreateEvent(k8sutil.NewErrorEvent("ArangoDeployment finalizer inspection failed", err, d.apiObject)) } @@ -86,11 +100,15 @@ func (d *Deployment) inspectDeployment(lastInterval util.Interval) util.Interval return nextInterval } - if inspectNextInterval, err := d.inspectDeploymentWithError(ctx, nextInterval); err != nil { - nextInterval = inspectNextInterval - hasError = true + if inspectNextInterval, err := d.inspectDeploymentWithError(ctx, nextInterval, cachedStatus); err != nil { + if !operatorErrors.IsReconcile(err) { + nextInterval = inspectNextInterval + hasError = true - d.CreateEvent(k8sutil.NewErrorEvent("Reconcilation failed", err, d.apiObject)) + d.CreateEvent(k8sutil.NewErrorEvent("Reconcilation failed", err, d.apiObject)) + } else { + nextInterval = minInspectionInterval + } } } @@ -106,7 +124,12 @@ func (d *Deployment) inspectDeployment(lastInterval util.Interval) util.Interval return nextInterval.ReduceTo(maxInspectionInterval) } -func (d *Deployment) inspectDeploymentWithError(ctx context.Context, lastInterval util.Interval) (nextInterval util.Interval, inspectError error) { +func (d *Deployment) inspectDeploymentWithError(ctx context.Context, lastInterval util.Interval, cachedStatus inspector.Inspector) (nextInterval util.Interval, inspectError error) { + t := time.Now() + defer func() { + d.deps.Log.Info().Msgf("Reconciliation loop took %s", time.Since(t)) + }() + // Ensure that spec and status checksum are same spec := d.GetSpec() status, _ := d.getStatus() @@ -128,13 +151,21 @@ func (d *Deployment) inspectDeploymentWithError(ctx context.Context, lastInterva } } + if err := d.resources.EnsureSecrets(cachedStatus); err != nil { + return minInspectionInterval, errors.Wrapf(err, "Secret creation failed") + } + + if err := d.resources.EnsureServices(cachedStatus); err != nil { + return minInspectionInterval, errors.Wrapf(err, "Service creation failed") + } + // Inspect secret hashes - if err := d.resources.ValidateSecretHashes(); err != nil { + if err := d.resources.ValidateSecretHashes(cachedStatus); err != nil { return minInspectionInterval, errors.Wrapf(err, "Secret hash validation failed") } // Check for LicenseKeySecret - if err := d.resources.ValidateLicenseKeySecret(); err != nil { + if err := d.resources.ValidateLicenseKeySecret(cachedStatus); err != nil { return minInspectionInterval, errors.Wrapf(err, "License Key Secret invalid") } @@ -144,42 +175,43 @@ func (d *Deployment) inspectDeploymentWithError(ctx context.Context, lastInterva } // Ensure we have image info - if retrySoon, err := d.ensureImages(d.apiObject); err != nil { + if retrySoon, err := d.ensureImages(d.apiObject); err != nil { return minInspectionInterval, errors.Wrapf(err, "Image detection failed") } else if retrySoon { return minInspectionInterval, nil } // Inspection of generated resources needed - if x, err := d.resources.InspectPods(ctx); err != nil { + if x, err := d.resources.InspectPods(ctx, cachedStatus); err != nil { return minInspectionInterval, errors.Wrapf(err, "Pod inspection failed") } else { nextInterval = nextInterval.ReduceTo(x) } - if x, err := d.resources.InspectPVCs(ctx); err != nil { + + if x, err := d.resources.InspectPVCs(ctx, cachedStatus); err != nil { return minInspectionInterval, errors.Wrapf(err, "PVC inspection failed") } else { nextInterval = nextInterval.ReduceTo(x) } // Check members for resilience - if err := d.resilience.CheckMemberFailure(); err != nil { + if err := d.resilience.CheckMemberFailure(); err != nil { return minInspectionInterval, errors.Wrapf(err, "Member failure detection failed") } // Immediate actions - if err := d.reconciler.CheckDeployment(); err != nil { + if err := d.reconciler.CheckDeployment(); err != nil { return minInspectionInterval, errors.Wrapf(err, "Reconciler immediate actions failed") } - if interval, err := d.ensureResources(nextInterval); err != nil { + if interval, err := d.ensureResources(nextInterval, cachedStatus); err != nil { return minInspectionInterval, errors.Wrapf(err, "Reconciler resource recreation failed") } else { nextInterval = interval } // Create scale/update plan - if err, updated := d.reconciler.CreatePlan(ctx); err != nil { + if err, updated := d.reconciler.CreatePlan(ctx, cachedStatus); err != nil { return minInspectionInterval, errors.Wrapf(err, "Plan creation failed") } else if updated { return minInspectionInterval, nil @@ -213,7 +245,7 @@ func (d *Deployment) inspectDeploymentWithError(ctx context.Context, lastInterva } // Execute current step of scale/update plan - retrySoon, err := d.reconciler.ExecutePlan(ctx) + retrySoon, err := d.reconciler.ExecutePlan(ctx, cachedStatus) if err != nil { return minInspectionInterval, errors.Wrapf(err, "Plan execution failed") } @@ -237,7 +269,7 @@ func (d *Deployment) inspectDeploymentWithError(ctx context.Context, lastInterva } // At the end of the inspect, we cleanup terminated pods. - if x, err := d.resources.CleanupTerminatedPods(); err != nil { + if x, err := d.resources.CleanupTerminatedPods(cachedStatus); err != nil { return minInspectionInterval, errors.Wrapf(err, "Pod cleanup failed") } else { nextInterval = nextInterval.ReduceTo(x) @@ -246,31 +278,27 @@ func (d *Deployment) inspectDeploymentWithError(ctx context.Context, lastInterva return } -func (d *Deployment) ensureResources(lastInterval util.Interval) (util.Interval, error) { +func (d *Deployment) ensureResources(lastInterval util.Interval, cachedStatus inspector.Inspector) (util.Interval, error) { // Ensure all resources are created - if err := d.resources.EnsureSecrets(); err != nil { - return minInspectionInterval, errors.Wrapf(err, "Secret creation failed") - } - if err := d.resources.EnsureServices(); err != nil { - return minInspectionInterval, errors.Wrapf(err, "Service creation failed") - } if d.haveServiceMonitorCRD { if err := d.resources.EnsureServiceMonitor(); err != nil { return minInspectionInterval, errors.Wrapf(err, "Service monitor creation failed") } } - if err := d.resources.EnsurePVCs(); err != nil { + if err := d.resources.EnsurePVCs(cachedStatus); err != nil { return minInspectionInterval, errors.Wrapf(err, "PVC creation failed") } - if err := d.resources.EnsurePods(); err != nil { + + if err := d.resources.EnsurePods(cachedStatus); err != nil { return minInspectionInterval, errors.Wrapf(err, "Pod creation failed") } - if err := d.resources.EnsurePDBs(); err != nil { + + if err := d.resources.EnsurePDBs(); err != nil { return minInspectionInterval, errors.Wrapf(err, "PDB creation failed") } - if err := d.resources.EnsureAnnotations(); err != nil { + if err := d.resources.EnsureAnnotations(cachedStatus); err != nil { return minInspectionInterval, errors.Wrapf(err, "Annotation update failed") } diff --git a/pkg/deployment/deployment_run_test.go b/pkg/deployment/deployment_run_test.go index adb146440..4dc68afb1 100644 --- a/pkg/deployment/deployment_run_test.go +++ b/pkg/deployment/deployment_run_test.go @@ -25,6 +25,7 @@ package deployment import ( "encoding/json" "fmt" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" "testing" "github.com/arangodb/kube-arangodb/pkg/util" @@ -53,7 +54,7 @@ func runTestCase(t *testing.T, testCase testCaseStruct) { // Arrange d, eventRecorder := createTestDeployment(testCase.config, testCase.ArangoDeployment) - err := d.resources.EnsureSecrets() + err := d.resources.EnsureSecrets(inspector.NewEmptyInspector()) require.NoError(t, err) if testCase.Helper != nil { @@ -69,7 +70,7 @@ func runTestCase(t *testing.T, testCase testCaseStruct) { } // Act - err = d.resources.EnsurePods() + err = d.resources.EnsurePods(inspector.NewEmptyInspector()) // Assert if testCase.ExpectedError != nil { diff --git a/pkg/deployment/images.go b/pkg/deployment/images.go index 489f7ed30..8a2a3658f 100644 --- a/pkg/deployment/images.go +++ b/pkg/deployment/images.go @@ -29,6 +29,9 @@ import ( "strings" "time" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/interfaces" + "github.com/arangodb/kube-arangodb/pkg/deployment/pod" "github.com/rs/zerolog" @@ -43,8 +46,8 @@ import ( "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" ) -var _ k8sutil.PodCreator = &ImageUpdatePod{} -var _ k8sutil.ContainerCreator = &ArangoDImageUpdateContainer{} +var _ interfaces.PodCreator = &ImageUpdatePod{} +var _ interfaces.ContainerCreator = &ArangoDImageUpdateContainer{} type ImageUpdatePod struct { spec api.DeploymentSpec @@ -275,7 +278,7 @@ func (i *ImageUpdatePod) GetImagePullSecrets() []string { return i.spec.ImagePullSecrets } -func (i *ImageUpdatePod) GetContainerCreator() k8sutil.ContainerCreator { +func (i *ImageUpdatePod) GetContainerCreator() interfaces.ContainerCreator { return &ArangoDImageUpdateContainer{ spec: i.spec, image: i.image, @@ -373,6 +376,6 @@ func (i *ImageUpdatePod) GetNodeAffinity() *core.NodeAffinity { return pod.ReturnNodeAffinityOrNil(a) } -func (i *ImageUpdatePod) Validate(secrets k8sutil.SecretInterface) error { +func (i *ImageUpdatePod) Validate(cachedStatus inspector.Inspector) error { return nil } diff --git a/pkg/deployment/pod/affinity.go b/pkg/deployment/pod/affinity.go index 290d224fe..bcdd9e158 100644 --- a/pkg/deployment/pod/affinity.go +++ b/pkg/deployment/pod/affinity.go @@ -24,11 +24,12 @@ package pod import ( "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/interfaces" core "k8s.io/api/core/v1" meta "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func AppendPodAntiAffinityDefault(p k8sutil.PodCreator, a *core.PodAntiAffinity) { +func AppendPodAntiAffinityDefault(p interfaces.PodCreator, a *core.PodAntiAffinity) { labels := k8sutil.LabelsForDeployment(p.GetName(), p.GetRole()) labelSelector := &meta.LabelSelector{ MatchLabels: labels, @@ -66,7 +67,7 @@ func AppendNodeSelector(a *core.NodeAffinity) { }) } -func AppendAffinityWithRole(p k8sutil.PodCreator, a *core.PodAffinity, role string) { +func AppendAffinityWithRole(p interfaces.PodCreator, a *core.PodAffinity, role string) { labelSelector := &meta.LabelSelector{ MatchLabels: k8sutil.LabelsForDeployment(p.GetName(), role), } diff --git a/pkg/deployment/pod/builder.go b/pkg/deployment/pod/builder.go index 64f906238..732b04da8 100644 --- a/pkg/deployment/pod/builder.go +++ b/pkg/deployment/pod/builder.go @@ -25,6 +25,7 @@ package pod import ( "github.com/arangodb/go-driver" deploymentApi "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" core "k8s.io/api/core/v1" meta "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -45,5 +46,5 @@ type Input struct { type Builder interface { Args(i Input) k8sutil.OptionPairs Volumes(i Input) ([]core.Volume, []core.VolumeMount) - Verify(i Input, s k8sutil.SecretInterface) error + Verify(i Input, cachedStatus inspector.Inspector) error } diff --git a/pkg/deployment/pod/encryption.go b/pkg/deployment/pod/encryption.go index 31ded381e..da8c2e4e1 100644 --- a/pkg/deployment/pod/encryption.go +++ b/pkg/deployment/pod/encryption.go @@ -27,6 +27,8 @@ import ( "fmt" "path/filepath" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" "github.com/arangodb/kube-arangodb/pkg/util/constants" "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" @@ -69,22 +71,28 @@ func GetEncryptionKey(secrets k8sutil.SecretInterface, name string) (string, []b return "", nil, false, errors.Wrapf(err, "Unable to fetch secret") } + sha, data, err := GetEncryptionKeyFromSecret(keyfile) + + return sha, data, true, err +} + +func GetEncryptionKeyFromSecret(keyfile *core.Secret) (string, []byte, error) { if len(keyfile.Data) == 0 { - return "", nil, false, nil + return "", nil, nil } d, ok := keyfile.Data[constants.SecretEncryptionKey] if !ok { - return "", nil, false, nil + return "", nil, nil } if len(d) != 32 { - return "", nil, false, errors.Errorf("Current encryption key is not valid") + return "", nil, errors.Errorf("Current encryption key is not valid") } sha := fmt.Sprintf("%0x", sha256.Sum256(d)) - return sha, d, true, nil + return sha, d, nil } func GetKeyfolderSecretName(name string) string { @@ -138,12 +146,18 @@ func (e encryption) Volumes(i Input) ([]core.Volume, []core.VolumeMount) { } } -func (e encryption) Verify(i Input, s k8sutil.SecretInterface) error { +func (e encryption) Verify(i Input, cachedStatus inspector.Inspector) error { if !IsEncryptionEnabled(i) { return nil } + + secret, exists := cachedStatus.Secret(i.Deployment.RocksDB.Encryption.GetKeySecretName()) + if !exists { + return errors.Errorf("Encryption key secret does not exist %s", i.Deployment.RocksDB.Encryption.GetKeySecretName()) + } + if !MultiFileMode(i) { - if err := k8sutil.ValidateEncryptionKeySecret(s, i.Deployment.RocksDB.Encryption.GetKeySecretName()); err != nil { + if err := k8sutil.ValidateEncryptionKeyFromSecret(secret); err != nil { return errors.Wrapf(err, "RocksDB encryption key secret validation failed") } return nil diff --git a/pkg/deployment/pod/jwt.go b/pkg/deployment/pod/jwt.go new file mode 100644 index 000000000..3556c4391 --- /dev/null +++ b/pkg/deployment/pod/jwt.go @@ -0,0 +1,61 @@ +// +// DISCLAIMER +// +// Copyright 2020 ArangoDB GmbH, Cologne, Germany +// +// 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. +// +// Copyright holder is ArangoDB GmbH, Cologne, Germany +// +// Author Adam Janikowski +// + +package pod + +import ( + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" + "github.com/pkg/errors" + core "k8s.io/api/core/v1" +) + +func JWT() Builder { + return jwt{} +} + +type jwt struct{} + +func (e jwt) Args(i Input) k8sutil.OptionPairs { + return nil +} + +func (e jwt) Volumes(i Input) ([]core.Volume, []core.VolumeMount) { + return nil, nil +} + +func (e jwt) Verify(i Input, cachedStatus inspector.Inspector) error { + if !i.Deployment.IsAuthenticated() { + return nil + } + + secret, exists := cachedStatus.Secret(i.Deployment.Authentication.GetJWTSecretName()) + if !exists { + return errors.Errorf("Secret for JWT token is missing %s", i.Deployment.Authentication.GetJWTSecretName()) + } + + if err := k8sutil.ValidateTokenFromSecret(secret); err != nil { + return errors.Wrapf(err, "Cluster JWT secret validation failed") + } + + return nil +} diff --git a/pkg/deployment/pod/sni.go b/pkg/deployment/pod/sni.go index 1a84e926f..9697a98d8 100644 --- a/pkg/deployment/pod/sni.go +++ b/pkg/deployment/pod/sni.go @@ -26,6 +26,8 @@ import ( "crypto/sha256" "fmt" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" "github.com/arangodb/kube-arangodb/pkg/util" @@ -33,7 +35,6 @@ import ( "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" "github.com/pkg/errors" core "k8s.io/api/core/v1" - meta "k8s.io/apimachinery/pkg/apis/meta/v1" ) func GroupSNISupported(mode api.DeploymentMode, group api.ServerGroup) bool { @@ -69,15 +70,15 @@ func (s sni) isSupported(i Input) bool { return GroupSNISupported(i.Deployment.Mode.Get(), i.Group) } -func (s sni) Verify(i Input, secrets k8sutil.SecretInterface) error { +func (s sni) Verify(i Input, cachedStatus inspector.Inspector) error { if !s.isSupported(i) { return nil } for _, secret := range util.SortKeys(i.Deployment.TLS.GetSNI().Mapping) { - kubeSecret, err := secrets.Get(secret, meta.GetOptions{}) - if err != nil { - return err + kubeSecret, exists := cachedStatus.Secret(secret) + if !exists { + return errors.Errorf("SNI Secret not found %s", secret) } _, ok := kubeSecret.Data[constants.SecretTLSKeyfile] diff --git a/pkg/deployment/pod/upgrade.go b/pkg/deployment/pod/upgrade.go index 562eb2d7c..67266a0bd 100644 --- a/pkg/deployment/pod/upgrade.go +++ b/pkg/deployment/pod/upgrade.go @@ -24,6 +24,7 @@ package pod import ( deploymentApi "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" core "k8s.io/api/core/v1" ) @@ -34,7 +35,7 @@ func AutoUpgrade() Builder { type autoUpgrade struct{} -func (u autoUpgrade) Verify(i Input, s k8sutil.SecretInterface) error { +func (u autoUpgrade) Verify(i Input, cachedStatus inspector.Inspector) error { return nil } diff --git a/pkg/deployment/reconcile/action_context.go b/pkg/deployment/reconcile/action_context.go index 6b302c86a..00d868f73 100644 --- a/pkg/deployment/reconcile/action_context.go +++ b/pkg/deployment/reconcile/action_context.go @@ -26,6 +26,8 @@ import ( "context" "fmt" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + backupApi "github.com/arangodb/kube-arangodb/pkg/apis/backup/v1" "github.com/arangodb/go-driver/agency" @@ -127,20 +129,28 @@ type ActionContext interface { GetBackup(backup string) (*backupApi.ArangoBackup, error) // GetName receives information about a deployment name GetName() string + // GetNameget current cached state of deployment + GetCachedStatus() inspector.Inspector } // newActionContext creates a new ActionContext implementation. -func newActionContext(log zerolog.Logger, context Context) ActionContext { +func newActionContext(log zerolog.Logger, context Context, cachedStatus inspector.Inspector) ActionContext { return &actionContext{ - log: log, - context: context, + log: log, + context: context, + cachedStatus: cachedStatus, } } // actionContext implements ActionContext type actionContext struct { - log zerolog.Logger - context Context + log zerolog.Logger + context Context + cachedStatus inspector.Inspector +} + +func (ac *actionContext) GetCachedStatus() inspector.Inspector { + return ac.cachedStatus } func (ac *actionContext) GetName() string { diff --git a/pkg/deployment/reconcile/action_tls_sni_update.go b/pkg/deployment/reconcile/action_tls_sni_update.go index 768e07b2b..8b61b3e0f 100644 --- a/pkg/deployment/reconcile/action_tls_sni_update.go +++ b/pkg/deployment/reconcile/action_tls_sni_update.go @@ -62,7 +62,7 @@ func (t *tlsSNIUpdate) CheckProgress(ctx context.Context) (bool, bool, error) { return true, false, nil } - fetchedSecrets, err := mapTLSSNIConfig(t.log, *sni, t.actionCtx.SecretsInterface()) + fetchedSecrets, err := mapTLSSNIConfig(t.log, *sni, t.actionCtx.GetCachedStatus()) if err != nil { t.log.Warn().Err(err).Msg("Unable to get SNI desired state") return true, false, nil diff --git a/pkg/deployment/reconcile/context.go b/pkg/deployment/reconcile/context.go index 3d612d4ec..1bec5dcf8 100644 --- a/pkg/deployment/reconcile/context.go +++ b/pkg/deployment/reconcile/context.go @@ -25,6 +25,8 @@ package reconcile import ( "context" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + backupApi "github.com/arangodb/kube-arangodb/pkg/apis/backup/v1" "github.com/arangodb/arangosync-client/client" @@ -76,8 +78,6 @@ type Context interface { // RemovePodFinalizers removes all the finalizers from the Pod with given name in the namespace // of the deployment. If the pod does not exist, the error is ignored. RemovePodFinalizers(podName string) error - // GetOwnedPods returns a list of all pods owned by the deployment. - GetOwnedPods() ([]v1.Pod, error) // UpdatePvc update PVC with given name in the namespace // of the deployment. UpdatePvc(pvc *v1.PersistentVolumeClaim) error @@ -91,9 +91,6 @@ type Context interface { // DeleteTLSKeyfile removes the Secret containing the TLS keyfile for the given member. // If the secret does not exist, the error is ignored. DeleteTLSKeyfile(group api.ServerGroup, member api.MemberStatus) error - // GetTLSCA returns the TLS CA certificate in the secret with given name. - // Returns: publicKey, privateKey, ownerByDeployment, error - GetTLSCA(secretName string) (string, string, bool, error) // DeleteSecret removes the Secret with given name. // If the secret does not exist, the error is ignored. DeleteSecret(secretName string) error @@ -110,7 +107,7 @@ type Context interface { // GetAgencyData object for key path GetAgencyData(ctx context.Context, i interface{}, keyParts ...string) error // Renders Pod definition for member - RenderPodForMember(spec api.DeploymentSpec, status api.DeploymentStatus, memberID string, imageInfo api.ImageInfo) (*v1.Pod, error) + RenderPodForMember(cachedStatus inspector.Inspector, spec api.DeploymentSpec, status api.DeploymentStatus, memberID string, imageInfo api.ImageInfo) (*v1.Pod, error) // SelectImage select currently used image by pod SelectImage(spec api.DeploymentSpec, status api.DeploymentStatus) (api.ImageInfo, bool) // WithStatusUpdate update status of ArangoDeployment with defined modifier. If action returns True action is taken diff --git a/pkg/deployment/reconcile/helper_tls_sni.go b/pkg/deployment/reconcile/helper_tls_sni.go index 409edc857..117c60ab5 100644 --- a/pkg/deployment/reconcile/helper_tls_sni.go +++ b/pkg/deployment/reconcile/helper_tls_sni.go @@ -30,14 +30,13 @@ import ( "github.com/arangodb/go-driver" api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" "github.com/arangodb/kube-arangodb/pkg/deployment/client" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" "github.com/arangodb/kube-arangodb/pkg/util/constants" - "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" "github.com/pkg/errors" "github.com/rs/zerolog" - meta "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func mapTLSSNIConfig(log zerolog.Logger, sni api.TLSSNISpec, secrets k8sutil.SecretInterface) (map[string]string, error) { +func mapTLSSNIConfig(log zerolog.Logger, sni api.TLSSNISpec, cachedStatus inspector.Inspector) (map[string]string, error) { fetchedSecrets := map[string]string{} mapping := sni.Mapping @@ -46,9 +45,9 @@ func mapTLSSNIConfig(log zerolog.Logger, sni api.TLSSNISpec, secrets k8sutil.Sec } for name, servers := range mapping { - secret, err := secrets.Get(name, meta.GetOptions{}) - if err != nil { - return nil, errors.WithMessage(err, "Unable to get SNI secret") + secret, exists := cachedStatus.Secret(name) + if !exists { + return nil, errors.Errorf("Secret %s does not exist", name) } tlsKey, ok := secret.Data[constants.SecretTLSKeyfile] diff --git a/pkg/deployment/reconcile/plan_builder.go b/pkg/deployment/reconcile/plan_builder.go index 556140caa..42d75b8ff 100644 --- a/pkg/deployment/reconcile/plan_builder.go +++ b/pkg/deployment/reconcile/plan_builder.go @@ -27,6 +27,8 @@ import ( "fmt" "time" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + "golang.org/x/net/context" "github.com/arangodb/kube-arangodb/pkg/deployment/agency" @@ -37,7 +39,6 @@ import ( api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" - v1 "k8s.io/api/core/v1" ) // upgradeDecision is the result of an upgrade check. @@ -54,20 +55,13 @@ type upgradeDecision struct { // CreatePlan considers the current specification & status of the deployment creates a plan to // get the status in line with the specification. // If a plan already exists, nothing is done. -func (d *Reconciler) CreatePlan(ctx context.Context) (error, bool) { - // Get all current pods - pods, err := d.context.GetOwnedPods() - if err != nil { - d.log.Debug().Err(err).Msg("Failed to get owned pods") - return maskAny(err), false - } - +func (d *Reconciler) CreatePlan(ctx context.Context, cachedStatus inspector.Inspector) (error, bool) { // Create plan apiObject := d.context.GetAPIObject() spec := d.context.GetSpec() status, lastVersion := d.context.GetStatus() builderCtx := newPlanBuilderContext(d.context) - newPlan, changed := createPlan(ctx, d.log, apiObject, status.Plan, spec, status, pods, builderCtx) + newPlan, changed := createPlan(ctx, d.log, apiObject, status.Plan, spec, status, cachedStatus, builderCtx) // If not change, we're done if !changed { @@ -112,7 +106,7 @@ func fetchAgency(ctx context.Context, log zerolog.Logger, // Otherwise the new plan is returned with a boolean true. func createPlan(ctx context.Context, log zerolog.Logger, apiObject k8sutil.APIObject, currentPlan api.Plan, spec api.DeploymentSpec, - status api.DeploymentStatus, pods []v1.Pod, + status api.DeploymentStatus, cachedStatus inspector.Inspector, builderCtx PlanBuilderContext) (api.Plan, bool) { if !currentPlan.IsEmpty() { @@ -121,13 +115,15 @@ func createPlan(ctx context.Context, log zerolog.Logger, apiObject k8sutil.APIOb } // Fetch agency plan - agencyPlan, agencyErr := fetchAgency(ctx, log, spec, status, builderCtx) + agencyPlan, agencyErr := fetchAgency(ctx, log, spec, status, builderCtx) // Check for various scenario's var plan api.Plan + pb := NewWithPlanBuilder(ctx, log, apiObject, spec, status, cachedStatus, builderCtx) + // Check for members in failed state - status.Members.ForeachServerGroup(func(group api.ServerGroup, members api.MemberStatusList) error { + status.Members.ForeachServerGroup(func(group api.ServerGroup, members api.MemberStatusList) error { for _, m := range members { if m.Phase != api.MemberPhaseFailed || len(plan) > 0 { continue @@ -184,7 +180,7 @@ func createPlan(ctx context.Context, log zerolog.Logger, apiObject k8sutil.APIOb } // Check for cleaned out dbserver in created state - for _, m := range status.Members.DBServers { + for _, m := range status.Members.DBServers { if plan.IsEmpty() && m.Phase.IsCreatedOrDrain() && m.Conditions.IsTrue(api.ConditionTypeCleanedOut) { log.Debug(). Str("id", m.ID). @@ -199,50 +195,44 @@ func createPlan(ctx context.Context, log zerolog.Logger, apiObject k8sutil.APIOb // Check for scale up/down if plan.IsEmpty() { - plan = createScaleMemeberPlan(log, spec, status) + plan = pb.Apply(createScaleMemeberPlan) } // Check for the need to rotate one or more members if plan.IsEmpty() { - newPlan, idle := createRotateOrUpgradePlan(log, apiObject, spec, status, builderCtx, pods) - if idle { - plan = append(plan, - api.NewAction(api.ActionTypeIdle, api.ServerGroupUnknown, "")) - } else { - plan = append(plan, newPlan...) - } + plan = pb.Apply(createRotateOrUpgradePlan) } // Add encryption keys if plan.IsEmpty() { - plan = createEncryptionKey(ctx, log, spec, status, builderCtx) + plan = pb.Apply(createEncryptionKey) } // Check for the need to rotate TLS certificate of a members if plan.IsEmpty() { - plan = createRotateTLSServerCertificatePlan(log, spec, status, builderCtx.GetTLSKeyfile) + plan = pb.Apply(createRotateTLSServerCertificatePlan) } // Check for changes storage classes or requirements if plan.IsEmpty() { - plan = createRotateServerStoragePlan(log, apiObject, spec, status, builderCtx.GetPvc, builderCtx.CreateEvent) + plan = pb.Apply(createRotateServerStoragePlan) } // Check for the need to rotate TLS CA certificate and all members if plan.IsEmpty() { - plan = createRotateTLSCAPlan(log, apiObject, spec, status, builderCtx.GetTLSCA, builderCtx.CreateEvent) + plan = pb.Apply(createRotateTLSCAPlan) } if plan.IsEmpty() { - plan = createRotateTLSServerSNIPlan(ctx, log, spec, status, builderCtx) + plan = pb.Apply(createRotateTLSServerSNIPlan) } if plan.IsEmpty() { - plan = createRestorePlan(ctx, log, spec, status, builderCtx) + plan = pb.Apply(createRestorePlan) } if plan.IsEmpty() { - plan = cleanEncryptionKey(ctx, log, spec, status, builderCtx) + plan = pb.Apply(cleanEncryptionKey) } // Return plan @@ -265,3 +255,41 @@ func createRotateMemberPlan(log zerolog.Logger, member api.MemberStatus, } return plan } + +type planBuilder func(ctx context.Context, + log zerolog.Logger, apiObject k8sutil.APIObject, + spec api.DeploymentSpec, status api.DeploymentStatus, + cachedStatus inspector.Inspector, context PlanBuilderContext) api.Plan + +func NewWithPlanBuilder(ctx context.Context, + log zerolog.Logger, apiObject k8sutil.APIObject, + spec api.DeploymentSpec, status api.DeploymentStatus, + cachedStatus inspector.Inspector, context PlanBuilderContext) WithPlanBuilder { + return &withPlanBuilder{ + ctx: ctx, + log: log, + apiObject: apiObject, + spec: spec, + status: status, + cachedStatus: cachedStatus, + context: context, + } +} + +type WithPlanBuilder interface { + Apply(p planBuilder) api.Plan +} + +type withPlanBuilder struct { + ctx context.Context + log zerolog.Logger + apiObject k8sutil.APIObject + spec api.DeploymentSpec + status api.DeploymentStatus + cachedStatus inspector.Inspector + context PlanBuilderContext +} + +func (w withPlanBuilder) Apply(p planBuilder) api.Plan { + return p(w.ctx, w.log, w.apiObject, w.spec, w.status, w.cachedStatus, w.context) +} diff --git a/pkg/deployment/reconcile/plan_builder_context.go b/pkg/deployment/reconcile/plan_builder_context.go index 0e9860afa..9c6f2d6a5 100644 --- a/pkg/deployment/reconcile/plan_builder_context.go +++ b/pkg/deployment/reconcile/plan_builder_context.go @@ -25,6 +25,8 @@ package reconcile import ( "context" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + backupApi "github.com/arangodb/kube-arangodb/pkg/apis/backup/v1" "github.com/arangodb/go-driver" @@ -39,9 +41,6 @@ type PlanBuilderContext interface { // GetTLSKeyfile returns the keyfile encoded TLS certificate+key for // the given member. GetTLSKeyfile(group api.ServerGroup, member api.MemberStatus) (string, error) - // GetTLSCA returns the TLS CA certificate in the secret with given name. - // Returns: publicKey, privateKey, ownerByDeployment, error - GetTLSCA(secretName string) (string, string, bool, error) // CreateEvent creates a given event. // On error, the error is logged. CreateEvent(evt *k8sutil.Event) @@ -56,7 +55,7 @@ type PlanBuilderContext interface { // GetAgencyData object for key path GetAgencyData(ctx context.Context, i interface{}, keyParts ...string) error // Renders Pod definition for member - RenderPodForMember(spec api.DeploymentSpec, status api.DeploymentStatus, memberID string, imageInfo api.ImageInfo) (*core.Pod, error) + RenderPodForMember(cachedStatus inspector.Inspector, spec api.DeploymentSpec, status api.DeploymentStatus, memberID string, imageInfo api.ImageInfo) (*core.Pod, error) // SelectImage select currently used image by pod SelectImage(spec api.DeploymentSpec, status api.DeploymentStatus) (api.ImageInfo, bool) // GetServerClient returns a cached client for a specific server. diff --git a/pkg/deployment/reconcile/plan_builder_encryption.go b/pkg/deployment/reconcile/plan_builder_encryption.go index 42a13e5e2..eb1d9d94d 100644 --- a/pkg/deployment/reconcile/plan_builder_encryption.go +++ b/pkg/deployment/reconcile/plan_builder_encryption.go @@ -25,18 +25,23 @@ package reconcile import ( "context" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" + + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + "github.com/arangodb/kube-arangodb/pkg/util" "github.com/arangodb/kube-arangodb/pkg/deployment/client" - meta "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/arangodb/kube-arangodb/pkg/deployment/pod" api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" "github.com/rs/zerolog" ) -func createEncryptionKey(ctx context.Context, log zerolog.Logger, spec api.DeploymentSpec, status api.DeploymentStatus, context PlanBuilderContext) api.Plan { +func createEncryptionKey(ctx context.Context, + log zerolog.Logger, apiObject k8sutil.APIObject, + spec api.DeploymentSpec, status api.DeploymentStatus, + cachedStatus inspector.Inspector, context PlanBuilderContext) api.Plan { if !spec.RocksDB.IsEncrypted() { return nil } @@ -45,9 +50,14 @@ func createEncryptionKey(ctx context.Context, log zerolog.Logger, spec api.Deplo return nil } - name, _, exists, err := pod.GetEncryptionKey(context.SecretsInterface(), spec.RocksDB.Encryption.GetKeySecretName()) + secret, exists := cachedStatus.Secret(spec.RocksDB.Encryption.GetKeySecretName()) + if !exists { + return nil + } + + name, _, err := pod.GetEncryptionKeyFromSecret(secret) if err != nil { - log.Err(err).Msgf("Unable to fetch encryption key") + log.Error().Err(err).Msgf("Unable to fetch encryption key") return nil } @@ -55,9 +65,9 @@ func createEncryptionKey(ctx context.Context, log zerolog.Logger, spec api.Deplo return nil } - keyfolder, err := context.SecretsInterface().Get(pod.GetKeyfolderSecretName(context.GetName()), meta.GetOptions{}) - if err != nil { - log.Err(err).Msgf("Unable to fetch encryption folder") + keyfolder, exists := cachedStatus.Secret(pod.GetKeyfolderSecretName(context.GetName())) + if !exists { + log.Error().Msgf("Encryption key folder does not exist") return nil } @@ -133,7 +143,10 @@ func createEncryptionKey(ctx context.Context, log zerolog.Logger, spec api.Deplo return api.Plan{} } -func cleanEncryptionKey(ctx context.Context, log zerolog.Logger, spec api.DeploymentSpec, status api.DeploymentStatus, context PlanBuilderContext) api.Plan { +func cleanEncryptionKey(ctx context.Context, + log zerolog.Logger, apiObject k8sutil.APIObject, + spec api.DeploymentSpec, status api.DeploymentStatus, + cachedStatus inspector.Inspector, context PlanBuilderContext) api.Plan { if !spec.RocksDB.IsEncrypted() { return nil } @@ -142,9 +155,9 @@ func cleanEncryptionKey(ctx context.Context, log zerolog.Logger, spec api.Deploy return nil } - keyfolder, err := context.SecretsInterface().Get(pod.GetKeyfolderSecretName(context.GetName()), meta.GetOptions{}) - if err != nil { - log.Err(err).Msgf("Unable to fetch encryption folder") + keyfolder, exists := cachedStatus.Secret(pod.GetKeyfolderSecretName(context.GetName())) + if !exists { + log.Error().Msgf("Encryption key folder does not exist") return nil } @@ -152,9 +165,13 @@ func cleanEncryptionKey(ctx context.Context, log zerolog.Logger, spec api.Deploy return nil } - name, _, exists, err := pod.GetEncryptionKey(context.SecretsInterface(), spec.RocksDB.Encryption.GetKeySecretName()) + secret, exists := cachedStatus.Secret(spec.RocksDB.Encryption.GetKeySecretName()) + if !exists { + return nil + } + + name, _, err := pod.GetEncryptionKeyFromSecret(secret) if err != nil { - log.Err(err).Msgf("Unable to fetch encryption key") return nil } diff --git a/pkg/deployment/reconcile/plan_builder_restore.go b/pkg/deployment/reconcile/plan_builder_restore.go index 4db9d297c..f13ffb325 100644 --- a/pkg/deployment/reconcile/plan_builder_restore.go +++ b/pkg/deployment/reconcile/plan_builder_restore.go @@ -25,6 +25,9 @@ package reconcile import ( "context" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" + backupv1 "github.com/arangodb/kube-arangodb/pkg/apis/backup/v1" "github.com/arangodb/kube-arangodb/pkg/deployment/pod" meta "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -33,7 +36,10 @@ import ( "github.com/rs/zerolog" ) -func createRestorePlan(ctx context.Context, log zerolog.Logger, spec api.DeploymentSpec, status api.DeploymentStatus, builderCtx PlanBuilderContext) api.Plan { +func createRestorePlan(ctx context.Context, + log zerolog.Logger, apiObject k8sutil.APIObject, + spec api.DeploymentSpec, status api.DeploymentStatus, + cachedStatus inspector.Inspector, context PlanBuilderContext) api.Plan { if spec.RestoreFrom == nil && status.Restore != nil { return api.Plan{ api.NewAction(api.ActionTypeBackupRestoreClean, api.ServerGroupUnknown, ""), @@ -41,13 +47,13 @@ func createRestorePlan(ctx context.Context, log zerolog.Logger, spec api.Deploym } if spec.RestoreFrom != nil && status.Restore == nil { - backup, err := builderCtx.GetBackup(spec.GetRestoreFrom()) + backup, err := context.GetBackup(spec.GetRestoreFrom()) if err != nil { log.Warn().Err(err).Msg("Backup not found") return nil } - if p := createRestorePlanEncryption(ctx, log, spec, status, builderCtx, backup); !p.IsEmpty() { + if p := createRestorePlanEncryption(ctx, log, spec, status, context, backup); !p.IsEmpty() { return p } diff --git a/pkg/deployment/reconcile/plan_builder_rotate_upgrade.go b/pkg/deployment/reconcile/plan_builder_rotate_upgrade.go index 2d7e59a99..79e541b67 100644 --- a/pkg/deployment/reconcile/plan_builder_rotate_upgrade.go +++ b/pkg/deployment/reconcile/plan_builder_rotate_upgrade.go @@ -23,10 +23,13 @@ package reconcile import ( + "context" + "github.com/arangodb/go-driver" upgraderules "github.com/arangodb/go-upgrade-rules" "github.com/arangodb/kube-arangodb/pkg/apis/deployment" api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" "github.com/arangodb/kube-arangodb/pkg/util" "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" "github.com/rs/zerolog" @@ -35,8 +38,25 @@ import ( ) // createRotateOrUpgradePlan goes over all pods to check if an upgrade or rotate is needed. -func createRotateOrUpgradePlan(log zerolog.Logger, apiObject k8sutil.APIObject, spec api.DeploymentSpec, - status api.DeploymentStatus, context PlanBuilderContext, pods []core.Pod) (api.Plan, bool) { + +func createRotateOrUpgradePlan(ctx context.Context, + log zerolog.Logger, apiObject k8sutil.APIObject, + spec api.DeploymentSpec, status api.DeploymentStatus, + cachedStatus inspector.Inspector, context PlanBuilderContext) api.Plan { + var plan api.Plan + + newPlan, idle := createRotateOrUpgradePlanInternal(log, apiObject, spec, status, cachedStatus, context) + if idle { + plan = append(plan, + api.NewAction(api.ActionTypeIdle, api.ServerGroupUnknown, "")) + } else { + plan = append(plan, newPlan...) + } + return plan +} + +func createRotateOrUpgradePlanInternal(log zerolog.Logger, apiObject k8sutil.APIObject, spec api.DeploymentSpec, + status api.DeploymentStatus, cachedStatus inspector.Inspector, context PlanBuilderContext) (api.Plan, bool) { var newPlan api.Plan var upgradeNotAllowed bool @@ -51,11 +71,12 @@ func createRotateOrUpgradePlan(log zerolog.Logger, apiObject k8sutil.APIObject, continue } - pod, found := k8sutil.GetPodByName(pods, m.PodName) + pod, found := cachedStatus.Pod(m.PodName) if !found { continue } + log.Debug().Msgf("Before upgrade") // Got pod, compare it with what it should be decision := podNeedsUpgrading(log, pod, spec, status.Images) if decision.UpgradeNeeded && !decision.UpgradeAllowed { @@ -68,22 +89,26 @@ func createRotateOrUpgradePlan(log zerolog.Logger, apiObject k8sutil.APIObject, return nil } + log.Debug().Msgf("After upgrade") + if !newPlan.IsEmpty() { // Only rotate/upgrade 1 pod at a time continue } + log.Debug().Msgf("Before rotate") if decision.UpgradeNeeded { // Yes, upgrade is needed (and allowed) newPlan = createUpgradeMemberPlan(log, m, group, "Version upgrade", spec.GetImage(), status, !decision.AutoUpgradeNeeded) } else { // Use new level of rotate logic - rotNeeded, reason := podNeedsRotation(log, pod, apiObject, spec, group, status, m, context) + rotNeeded, reason := podNeedsRotation(log, pod, apiObject, spec, group, status, m, cachedStatus, context) if rotNeeded { newPlan = createRotateMemberPlan(log, m, group, reason) } } + log.Debug().Msgf("After rotate") if !newPlan.IsEmpty() { // Only rotate/upgrade 1 pod at a time @@ -121,8 +146,8 @@ func createRotateOrUpgradePlan(log zerolog.Logger, apiObject k8sutil.APIObject, // podNeedsUpgrading decides if an upgrade of the pod is needed (to comply with // the given spec) and if that is allowed. -func podNeedsUpgrading(log zerolog.Logger, p core.Pod, spec api.DeploymentSpec, images api.ImageInfoList) upgradeDecision { - if c, found := k8sutil.GetContainerByName(&p, k8sutil.ServerContainerName); found { +func podNeedsUpgrading(log zerolog.Logger, p *core.Pod, spec api.DeploymentSpec, images api.ImageInfoList) upgradeDecision { + if c, found := k8sutil.GetContainerByName(p, k8sutil.ServerContainerName); found { specImageInfo, found := images.GetByImage(spec.GetImage()) if !found { return upgradeDecision{UpgradeNeeded: false} @@ -191,9 +216,9 @@ func podNeedsUpgrading(log zerolog.Logger, p core.Pod, spec api.DeploymentSpec, // given pod differs from what it should be according to the // given deployment spec. // When true is returned, a reason for the rotation is already returned. -func podNeedsRotation(log zerolog.Logger, p core.Pod, apiObject metav1.Object, spec api.DeploymentSpec, +func podNeedsRotation(log zerolog.Logger, p *core.Pod, apiObject metav1.Object, spec api.DeploymentSpec, group api.ServerGroup, status api.DeploymentStatus, m api.MemberStatus, - context PlanBuilderContext) (bool, string) { + cachedStatus inspector.Inspector, context PlanBuilderContext) (bool, string) { if m.PodUID != p.UID { return true, "Pod UID does not match, this pod is not managed by Operator. Recreating" } @@ -208,7 +233,7 @@ func podNeedsRotation(log zerolog.Logger, p core.Pod, apiObject metav1.Object, s return false, "" } - renderedPod, err := context.RenderPodForMember(spec, status, m.ID, imageInfo) + renderedPod, err := context.RenderPodForMember(cachedStatus, spec, status, m.ID, imageInfo) if err != nil { log.Err(err).Msg("Error while rendering pod") return false, "" diff --git a/pkg/deployment/reconcile/plan_builder_scale.go b/pkg/deployment/reconcile/plan_builder_scale.go index de233b027..9fdb4747c 100644 --- a/pkg/deployment/reconcile/plan_builder_scale.go +++ b/pkg/deployment/reconcile/plan_builder_scale.go @@ -23,11 +23,18 @@ package reconcile import ( + "context" + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" "github.com/rs/zerolog" ) -func createScaleMemeberPlan(log zerolog.Logger, spec api.DeploymentSpec, status api.DeploymentStatus) api.Plan { +func createScaleMemeberPlan(ctx context.Context, + log zerolog.Logger, apiObject k8sutil.APIObject, + spec api.DeploymentSpec, status api.DeploymentStatus, + cachedStatus inspector.Inspector, context PlanBuilderContext) api.Plan { var plan api.Plan diff --git a/pkg/deployment/reconcile/plan_builder_storage.go b/pkg/deployment/reconcile/plan_builder_storage.go index 9d69b7ddd..5869ca0a3 100644 --- a/pkg/deployment/reconcile/plan_builder_storage.go +++ b/pkg/deployment/reconcile/plan_builder_storage.go @@ -23,6 +23,9 @@ package reconcile import ( + "context" + + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" "github.com/rs/zerolog" core "k8s.io/api/core/v1" @@ -33,9 +36,10 @@ import ( // createRotateServerStoragePlan creates plan to rotate a server and its volume because of a // different storage class or a difference in storage resource requirements. -func createRotateServerStoragePlan(log zerolog.Logger, apiObject k8sutil.APIObject, spec api.DeploymentSpec, status api.DeploymentStatus, - getPVC func(pvcName string) (*core.PersistentVolumeClaim, error), - createEvent func(evt *k8sutil.Event)) api.Plan { +func createRotateServerStoragePlan(ctx context.Context, + log zerolog.Logger, apiObject k8sutil.APIObject, + spec api.DeploymentSpec, status api.DeploymentStatus, + cachedStatus inspector.Inspector, context PlanBuilderContext) api.Plan { if spec.GetMode() == api.DeploymentModeSingle { // Storage cannot be changed in single server deployments return nil @@ -58,9 +62,9 @@ func createRotateServerStoragePlan(log zerolog.Logger, apiObject k8sutil.APIObje groupSpec := spec.GetServerGroupSpec(group) storageClassName := groupSpec.GetStorageClassName() // Load PVC - pvc, err := getPVC(m.PersistentVolumeClaimName) - if err != nil { - log.Warn().Err(err). + pvc, exists := cachedStatus.PersistentVolumeClaim(m.PersistentVolumeClaimName) + if !exists { + log.Warn(). Str("role", group.AsRole()). Str("id", m.ID). Msg("Failed to get PVC") @@ -92,7 +96,7 @@ func createRotateServerStoragePlan(log zerolog.Logger, apiObject k8sutil.APIObje ) } else { // Only agents & dbservers are allowed to change their storage class. - createEvent(k8sutil.NewCannotChangeStorageClassEvent(apiObject, m.ID, group.AsRole(), "Not supported")) + context.CreateEvent(k8sutil.NewCannotChangeStorageClassEvent(apiObject, m.ID, group.AsRole(), "Not supported")) } } else if k8sutil.IsPersistentVolumeClaimFileSystemResizePending(pvc) { // rotation needed diff --git a/pkg/deployment/reconcile/plan_builder_test.go b/pkg/deployment/reconcile/plan_builder_test.go index 77eb6e2e1..dca97d25d 100644 --- a/pkg/deployment/reconcile/plan_builder_test.go +++ b/pkg/deployment/reconcile/plan_builder_test.go @@ -24,11 +24,12 @@ package reconcile import ( "context" - "errors" "fmt" "io/ioutil" "testing" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + backupApi "github.com/arangodb/kube-arangodb/pkg/apis/backup/v1" "github.com/arangodb/arangosync-client/client" @@ -36,7 +37,7 @@ import ( "github.com/rs/zerolog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + meta "k8s.io/apimachinery/pkg/apis/meta/v1" driver "github.com/arangodb/go-driver" api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" @@ -57,6 +58,10 @@ type testContext struct { RecordedEvent *k8sutil.Event } +func (c *testContext) RenderPodForMember(cachedStatus inspector.Inspector, spec api.DeploymentSpec, status api.DeploymentStatus, memberID string, imageInfo api.ImageInfo) (*core.Pod, error) { + panic("implement me") +} + func (c *testContext) GetName() string { panic("implement me") } @@ -73,10 +78,6 @@ func (c *testContext) WithStatusUpdate(action func(s *api.DeploymentStatus) bool panic("implement me") } -func (c *testContext) RenderPodForMember(spec api.DeploymentSpec, status api.DeploymentStatus, memberID string, imageInfo api.ImageInfo) (*core.Pod, error) { - panic("implement me") -} - func (c *testContext) SelectImage(spec api.DeploymentSpec, status api.DeploymentStatus) (api.ImageInfo, bool) { panic("implement me") } @@ -204,7 +205,7 @@ func (c *testContext) GetPvc(pvcName string) (*core.PersistentVolumeClaim, error } // GetExpectedPodArguments creates command line arguments for a server in the given group with given ID. -func (c *testContext) GetExpectedPodArguments(apiObject metav1.Object, deplSpec api.DeploymentSpec, group api.ServerGroup, +func (c *testContext) GetExpectedPodArguments(apiObject meta.Object, deplSpec api.DeploymentSpec, group api.ServerGroup, agents api.MemberStatusList, id string, version driver.Version) []string { return nil // not implemented } @@ -250,7 +251,7 @@ func TestCreatePlanSingleScale(t *testing.T) { } spec.SetDefaults("test") depl := &api.ArangoDeployment{ - ObjectMeta: metav1.ObjectMeta{ + ObjectMeta: meta.ObjectMeta{ Name: "test_depl", Namespace: "test", }, @@ -259,7 +260,7 @@ func TestCreatePlanSingleScale(t *testing.T) { // Test with empty status var status api.DeploymentStatus - newPlan, changed := createPlan(ctx, log, depl, nil, spec, status, nil, c) + newPlan, changed := createPlan(ctx, log, depl, nil, spec, status, inspector.NewEmptyInspector(), c) assert.True(t, changed) assert.Len(t, newPlan, 0) // Single mode does not scale @@ -270,7 +271,7 @@ func TestCreatePlanSingleScale(t *testing.T) { PodName: "something", }, } - newPlan, changed = createPlan(ctx, log, depl, nil, spec, status, nil, c) + newPlan, changed = createPlan(ctx, log, depl, nil, spec, status, inspector.NewEmptyInspector(), c) assert.True(t, changed) assert.Len(t, newPlan, 0) // Single mode does not scale @@ -285,7 +286,7 @@ func TestCreatePlanSingleScale(t *testing.T) { PodName: "something1", }, } - newPlan, changed = createPlan(ctx, log, depl, nil, spec, status, nil, c) + newPlan, changed = createPlan(ctx, log, depl, nil, spec, status, inspector.NewEmptyInspector(), c) assert.True(t, changed) assert.Len(t, newPlan, 0) // Single mode does not scale } @@ -303,7 +304,7 @@ func TestCreatePlanActiveFailoverScale(t *testing.T) { spec.SetDefaults("test") spec.Single.Count = util.NewInt(2) depl := &api.ArangoDeployment{ - ObjectMeta: metav1.ObjectMeta{ + ObjectMeta: meta.ObjectMeta{ Name: "test_depl", Namespace: "test", }, @@ -314,7 +315,7 @@ func TestCreatePlanActiveFailoverScale(t *testing.T) { var status api.DeploymentStatus addAgentsToStatus(t, &status, 3) - newPlan, changed := createPlan(ctx, log, depl, nil, spec, status, nil, c) + newPlan, changed := createPlan(ctx, log, depl, nil, spec, status, inspector.NewEmptyInspector(), c) assert.True(t, changed) require.Len(t, newPlan, 2) assert.Equal(t, api.ActionTypeAddMember, newPlan[0].Type) @@ -327,7 +328,7 @@ func TestCreatePlanActiveFailoverScale(t *testing.T) { PodName: "something", }, } - newPlan, changed = createPlan(ctx, log, depl, nil, spec, status, nil, c) + newPlan, changed = createPlan(ctx, log, depl, nil, spec, status, inspector.NewEmptyInspector(), c) assert.True(t, changed) require.Len(t, newPlan, 1) assert.Equal(t, api.ActionTypeAddMember, newPlan[0].Type) @@ -352,7 +353,7 @@ func TestCreatePlanActiveFailoverScale(t *testing.T) { PodName: "something4", }, } - newPlan, changed = createPlan(ctx, log, depl, nil, spec, status, nil, c) + newPlan, changed = createPlan(ctx, log, depl, nil, spec, status, inspector.NewEmptyInspector(), c) assert.True(t, changed) require.Len(t, newPlan, 2) // Note: Downscaling is only down 1 at a time assert.Equal(t, api.ActionTypeShutdownMember, newPlan[0].Type) @@ -373,7 +374,7 @@ func TestCreatePlanClusterScale(t *testing.T) { } spec.SetDefaults("test") depl := &api.ArangoDeployment{ - ObjectMeta: metav1.ObjectMeta{ + ObjectMeta: meta.ObjectMeta{ Name: "test_depl", Namespace: "test", }, @@ -417,7 +418,7 @@ func TestCreatePlanClusterScale(t *testing.T) { PodName: "coordinator1", }, } - newPlan, changed = createPlan(ctx, log, depl, nil, spec, status, nil, c) + newPlan, changed = createPlan(ctx, log, depl, nil, spec, status, inspector.NewEmptyInspector(), c) assert.True(t, changed) require.Len(t, newPlan, 3) assert.Equal(t, api.ActionTypeAddMember, newPlan[0].Type) @@ -454,7 +455,7 @@ func TestCreatePlanClusterScale(t *testing.T) { } spec.DBServers.Count = util.NewInt(1) spec.Coordinators.Count = util.NewInt(1) - newPlan, changed = createPlan(ctx, log, depl, nil, spec, status, nil, c) + newPlan, changed = createPlan(ctx, log, depl, nil, spec, status, inspector.NewEmptyInspector(), c) assert.True(t, changed) require.Len(t, newPlan, 5) // Note: Downscaling is done 1 at a time assert.Equal(t, api.ActionTypeCleanOutMember, newPlan[0].Type) @@ -503,7 +504,7 @@ func TestCreatePlan(t *testing.T) { } deploymentTemplate := &api.ArangoDeployment{ - ObjectMeta: metav1.ObjectMeta{ + ObjectMeta: meta.ObjectMeta{ Name: "test_depl", Namespace: "test", }, @@ -531,15 +532,12 @@ func TestCreatePlan(t *testing.T) { ExpectedPlan api.Plan ExpectedLog string ExpectedEvent *k8sutil.Event + + Pods map[string]*core.Pod + Secrets map[string]*core.Secret + Services map[string]*core.Service + PVCS map[string]*core.PersistentVolumeClaim }{ - { - Name: "Can not get pods", - context: &testContext{ - ErrPods: errors.New("fake error"), - }, - ExpectedError: errors.New("fake error"), - ExpectedLog: "Failed to get owned pods", - }, { Name: "Can not create plan for single deployment", context: &testContext{ @@ -571,28 +569,18 @@ func TestCreatePlan(t *testing.T) { }, ExpectedPlan: []api.Action{}, }, - { - Name: "Getting PVC from kubernetes failed", - context: &testContext{ - ArangoDeployment: deploymentTemplate.DeepCopy(), - PVCErr: errors.New("fake error"), - }, - Helper: func(ad *api.ArangoDeployment) { - ad.Status.Members.DBServers[0].Phase = api.MemberPhaseCreated - ad.Status.Members.DBServers[0].PersistentVolumeClaimName = "pvc_test" - }, - ExpectedLog: "Failed to get PVC", - }, { Name: "Change Storage for DBServers", - context: &testContext{ - ArangoDeployment: deploymentTemplate.DeepCopy(), - PVC: &core.PersistentVolumeClaim{ + PVCS: map[string]*core.PersistentVolumeClaim{ + "pvc_test": { Spec: core.PersistentVolumeClaimSpec{ StorageClassName: util.NewString("oldStorage"), }, }, }, + context: &testContext{ + ArangoDeployment: deploymentTemplate.DeepCopy(), + }, Helper: func(ad *api.ArangoDeployment) { ad.Spec.DBServers = api.ServerGroupSpec{ Count: util.NewInt(3), @@ -618,14 +606,16 @@ func TestCreatePlan(t *testing.T) { }, { Name: "Change Storage for Agents with deprecated storage class name", - context: &testContext{ - ArangoDeployment: deploymentTemplate.DeepCopy(), - PVC: &core.PersistentVolumeClaim{ + PVCS: map[string]*core.PersistentVolumeClaim{ + "pvc_test":{ Spec: core.PersistentVolumeClaimSpec{ StorageClassName: util.NewString("oldStorage"), }, }, }, + context: &testContext{ + ArangoDeployment: deploymentTemplate.DeepCopy(), + }, Helper: func(ad *api.ArangoDeployment) { ad.Spec.Agents = api.ServerGroupSpec{ Count: util.NewInt(2), @@ -644,14 +634,16 @@ func TestCreatePlan(t *testing.T) { }, { Name: "Storage for Coordinators is not possible", - context: &testContext{ - ArangoDeployment: deploymentTemplate.DeepCopy(), - PVC: &core.PersistentVolumeClaim{ + PVCS: map[string]*core.PersistentVolumeClaim{ + "pvc_test": { Spec: core.PersistentVolumeClaimSpec{ StorageClassName: util.NewString("oldStorage"), }, }, }, + context: &testContext{ + ArangoDeployment: deploymentTemplate.DeepCopy(), + }, Helper: func(ad *api.ArangoDeployment) { ad.Spec.Coordinators = api.ServerGroupSpec{ Count: util.NewInt(3), @@ -674,9 +666,8 @@ func TestCreatePlan(t *testing.T) { }, { Name: "Create rotation plan", - context: &testContext{ - ArangoDeployment: deploymentTemplate.DeepCopy(), - PVC: &core.PersistentVolumeClaim{ + PVCS: map[string]*core.PersistentVolumeClaim{ + "pvc_test": { Spec: core.PersistentVolumeClaimSpec{ StorageClassName: util.NewString("oldStorage"), }, @@ -690,6 +681,9 @@ func TestCreatePlan(t *testing.T) { }, }, }, + context: &testContext{ + ArangoDeployment: deploymentTemplate.DeepCopy(), + }, Helper: func(ad *api.ArangoDeployment) { ad.Spec.Agents = api.ServerGroupSpec{ Count: util.NewInt(2), @@ -802,7 +796,7 @@ func TestCreatePlan(t *testing.T) { if testCase.Helper != nil { testCase.Helper(testCase.context.ArangoDeployment) } - err, _ := r.CreatePlan(ctx) + err, _ := r.CreatePlan(ctx, inspector.NewInspectorFromData(testCase.Pods, testCase.Secrets, testCase.PVCS, testCase.Services)) // Assert if testCase.ExpectedEvent != nil { diff --git a/pkg/deployment/reconcile/plan_builder_tls.go b/pkg/deployment/reconcile/plan_builder_tls.go index 8af831cdf..0976569ad 100644 --- a/pkg/deployment/reconcile/plan_builder_tls.go +++ b/pkg/deployment/reconcile/plan_builder_tls.go @@ -23,19 +23,24 @@ package reconcile import ( + "context" "crypto/x509" "encoding/pem" "net" "time" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" "github.com/rs/zerolog" ) // createRotateTLSServerCertificatePlan creates plan to rotate a server because of an (soon to be) expired TLS certificate. -func createRotateTLSServerCertificatePlan(log zerolog.Logger, spec api.DeploymentSpec, status api.DeploymentStatus, - getTLSKeyfile func(group api.ServerGroup, member api.MemberStatus) (string, error)) api.Plan { +func createRotateTLSServerCertificatePlan(ctx context.Context, + log zerolog.Logger, apiObject k8sutil.APIObject, + spec api.DeploymentSpec, status api.DeploymentStatus, + cachedStatus inspector.Inspector, context PlanBuilderContext) api.Plan { if !spec.TLS.IsSecure() { return nil } @@ -55,14 +60,24 @@ func createRotateTLSServerCertificatePlan(log zerolog.Logger, spec api.Deploymen continue } // Load keyfile - keyfile, err := getTLSKeyfile(group, m) + secret, exists := cachedStatus.Secret(k8sutil.CreateTLSKeyfileSecretName(context.GetName(), group.AsRole(), m.ID)) + if !exists { + log.Warn(). + Str("role", group.AsRole()). + Str("id", m.ID). + Msg("Failed to get TLS secret") + continue + } + + keyfile, err := k8sutil.GetTLSKeyfileFromSecret(secret) if err != nil { log.Warn().Err(err). Str("role", group.AsRole()). Str("id", m.ID). - Msg("Failed to get TLS secret") + Msg("Failed to parse TLS secret") continue } + tlsSpec := spec.TLS if group.IsArangosync() { tlsSpec = spec.Sync.TLS @@ -81,15 +96,25 @@ func createRotateTLSServerCertificatePlan(log zerolog.Logger, spec api.Deploymen } // createRotateTLSCAPlan creates plan to replace a TLS CA and rotate all server. -func createRotateTLSCAPlan(log zerolog.Logger, apiObject k8sutil.APIObject, +func createRotateTLSCAPlan(ctx context.Context, + log zerolog.Logger, apiObject k8sutil.APIObject, spec api.DeploymentSpec, status api.DeploymentStatus, - getTLSCA func(string) (string, string, bool, error), - createEvent func(evt *k8sutil.Event)) api.Plan { + cachedStatus inspector.Inspector, context PlanBuilderContext) api.Plan { if !spec.TLS.IsSecure() { return nil } + + asOwner := apiObject.AsOwner() + secretName := spec.TLS.GetCASecretName() - cert, _, isOwned, err := getTLSCA(secretName) + secret, exists := cachedStatus.Secret(secretName) + + if !exists { + log.Warn().Str("secret-name", secretName).Msg("TLS CA secret missing") + return nil + } + + cert, _, isOwned, err := k8sutil.GetCAFromSecret(secret, &asOwner) if err != nil { log.Warn().Err(err).Str("secret-name", secretName).Msg("Failed to fetch TLS CA secret") return nil @@ -129,7 +154,7 @@ func createRotateTLSCAPlan(log zerolog.Logger, apiObject k8sutil.APIObject, } else { // Rotating the CA results in downtime. // That is currently not allowed. - createEvent(k8sutil.NewDowntimeNotAllowedEvent(apiObject, "Rotate TLS CA")) + context.CreateEvent(k8sutil.NewDowntimeNotAllowedEvent(apiObject, "Rotate TLS CA")) } } return plan diff --git a/pkg/deployment/reconcile/plan_builder_tls_sni.go b/pkg/deployment/reconcile/plan_builder_tls_sni.go index eff93082e..42702f0e3 100644 --- a/pkg/deployment/reconcile/plan_builder_tls_sni.go +++ b/pkg/deployment/reconcile/plan_builder_tls_sni.go @@ -25,13 +25,20 @@ package reconcile import ( "context" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" + + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + "github.com/arangodb/kube-arangodb/pkg/deployment/pod" api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" "github.com/rs/zerolog" ) -func createRotateTLSServerSNIPlan(ctx context.Context, log zerolog.Logger, spec api.DeploymentSpec, status api.DeploymentStatus, context PlanBuilderContext) api.Plan { +func createRotateTLSServerSNIPlan(ctx context.Context, + log zerolog.Logger, apiObject k8sutil.APIObject, + spec api.DeploymentSpec, status api.DeploymentStatus, + cachedStatus inspector.Inspector, context PlanBuilderContext) api.Plan { if !spec.TLS.IsSecure() { return nil } @@ -45,7 +52,7 @@ func createRotateTLSServerSNIPlan(ctx context.Context, log zerolog.Logger, spec return nil } - fetchedSecrets, err := mapTLSSNIConfig(log, *sni, context.SecretsInterface()) + fetchedSecrets, err := mapTLSSNIConfig(log, *sni, cachedStatus) if err != nil { log.Warn().Err(err).Msg("Unable to get SNI desired state") return nil diff --git a/pkg/deployment/reconcile/plan_executor.go b/pkg/deployment/reconcile/plan_executor.go index 0f0c7acc5..d663d5d7b 100644 --- a/pkg/deployment/reconcile/plan_executor.go +++ b/pkg/deployment/reconcile/plan_executor.go @@ -27,6 +27,8 @@ import ( "fmt" "time" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + "github.com/rs/zerolog" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -37,7 +39,7 @@ import ( // ExecutePlan tries to execute the plan as far as possible. // Returns true when it has to be called again soon. // False otherwise. -func (d *Reconciler) ExecutePlan(ctx context.Context) (bool, error) { +func (d *Reconciler) ExecutePlan(ctx context.Context, cachedStatus inspector.Inspector) (bool, error) { log := d.log firstLoop := true @@ -61,7 +63,7 @@ func (d *Reconciler) ExecutePlan(ctx context.Context) (bool, error) { Str("group", planAction.Group.AsRole()). Str("member-id", planAction.MemberID). Logger() - action := d.createAction(ctx, log, planAction) + action := d.createAction(ctx, log, planAction, cachedStatus) if planAction.StartTime.IsZero() { // Not started yet ready, err := action.Start(ctx) @@ -158,8 +160,8 @@ func (d *Reconciler) ExecutePlan(ctx context.Context) (bool, error) { } // createAction create action object based on action type -func (d *Reconciler) createAction(ctx context.Context, log zerolog.Logger, action api.Action) Action { - actionCtx := newActionContext(log, d.context) +func (d *Reconciler) createAction(ctx context.Context, log zerolog.Logger, action api.Action, cachedStatus inspector.Inspector) Action { + actionCtx := newActionContext(log, d.context, cachedStatus) f, ok := getActionFactory(action.Type) if !ok { diff --git a/pkg/deployment/resources/annotations.go b/pkg/deployment/resources/annotations.go index b9229d563..7fd06b809 100644 --- a/pkg/deployment/resources/annotations.go +++ b/pkg/deployment/resources/annotations.go @@ -25,6 +25,8 @@ package resources import ( "time" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + "github.com/arangodb/kube-arangodb/pkg/apis/deployment" api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" "github.com/arangodb/kube-arangodb/pkg/backup/utils" @@ -37,12 +39,13 @@ import ( policyTyped "k8s.io/client-go/kubernetes/typed/policy/v1beta1" ) -func (r *Resources) EnsureAnnotations() error { +func (r *Resources) EnsureAnnotations(cachedStatus inspector.Inspector) error { kubecli := r.context.GetKubeCli() log.Info().Msgf("Ensuring annotations") if err := ensureSecretsAnnotations(kubecli.CoreV1().Secrets(r.context.GetNamespace()), + cachedStatus, deployment.ArangoDeploymentResourceKind, r.context.GetAPIObject().GetName(), r.context.GetAPIObject().GetNamespace(), @@ -59,6 +62,7 @@ func (r *Resources) EnsureAnnotations() error { } if err := ensureServicesAnnotations(kubecli.CoreV1().Services(r.context.GetNamespace()), + cachedStatus, deployment.ArangoDeploymentResourceKind, r.context.GetAPIObject().GetName(), r.context.GetAPIObject().GetNamespace(), @@ -75,6 +79,7 @@ func (r *Resources) EnsureAnnotations() error { } if err := ensurePvcsAnnotations(kubecli.CoreV1().PersistentVolumeClaims(r.context.GetNamespace()), + cachedStatus, deployment.ArangoDeploymentResourceKind, r.context.GetAPIObject().GetName(), r.context.GetAPIObject().GetNamespace(), @@ -83,6 +88,7 @@ func (r *Resources) EnsureAnnotations() error { } if err := ensurePodsAnnotations(kubecli.CoreV1().Pods(r.context.GetNamespace()), + cachedStatus, deployment.ArangoDeploymentResourceKind, r.context.GetAPIObject().GetName(), r.context.GetAPIObject().GetNamespace(), @@ -94,22 +100,20 @@ func (r *Resources) EnsureAnnotations() error { return nil } -func ensureSecretsAnnotations(client typedCore.SecretInterface, kind, name, namespace string, annotations map[string]string) error { - secrets, err := k8sutil.GetSecretsForParent(client, - kind, - name, - namespace) - if err != nil { - return err - } - - for _, secret := range secrets { +func ensureSecretsAnnotations(client typedCore.SecretInterface, cachedStatus inspector.Inspector, kind, name, namespace string, annotations map[string]string) error { + if err := cachedStatus.IterateSecrets(func(secret *core.Secret) error { if !k8sutil.CompareAnnotations(secret.GetAnnotations(), annotations) { log.Info().Msgf("Replacing annotations for Secret %s", secret.Name) - if err = setSecretAnnotations(client, secret, annotations); err != nil { + if err := setSecretAnnotations(client, secret, annotations); err != nil { return err } } + + return nil + }, func(secret *core.Secret) bool { + return k8sutil.IsChildResource(kind, name, namespace, secret) + }); err != nil { + return err } return nil @@ -172,22 +176,20 @@ func setServiceAccountAnnotations(client typedCore.ServiceAccountInterface, serv }) } -func ensureServicesAnnotations(client typedCore.ServiceInterface, kind, name, namespace string, annotations map[string]string) error { - services, err := k8sutil.GetServicesForParent(client, - kind, - name, - namespace) - if err != nil { - return err - } - - for _, service := range services { +func ensureServicesAnnotations(client typedCore.ServiceInterface, cachedStatus inspector.Inspector, kind, name, namespace string, annotations map[string]string) error { + if err := cachedStatus.IterateServices(func(service *core.Service) error { if !k8sutil.CompareAnnotations(service.GetAnnotations(), annotations) { log.Info().Msgf("Replacing annotations for Service %s", service.Name) - if err = setServiceAnnotations(client, service, annotations); err != nil { + if err := setServiceAnnotations(client, service, annotations); err != nil { return err } } + + return nil + }, func(service *core.Service) bool { + return k8sutil.IsChildResource(kind, name, namespace, service) + }); err != nil { + return err } return nil @@ -250,22 +252,20 @@ func setPdbAnnotations(client policyTyped.PodDisruptionBudgetInterface, podDisru }) } -func ensurePvcsAnnotations(client typedCore.PersistentVolumeClaimInterface, kind, name, namespace string, annotations map[string]string) error { - persistentVolumeClaims, err := k8sutil.GetPVCForParent(client, - kind, - name, - namespace) - if err != nil { - return err - } - - for _, persistentVolumeClaim := range persistentVolumeClaims { +func ensurePvcsAnnotations(client typedCore.PersistentVolumeClaimInterface, cachedStatus inspector.Inspector, kind, name, namespace string, annotations map[string]string) error { + if err := cachedStatus.IteratePersistentVolumeClaims(func(persistentVolumeClaim *core.PersistentVolumeClaim) error { if !k8sutil.CompareAnnotations(persistentVolumeClaim.GetAnnotations(), annotations) { log.Info().Msgf("Replacing annotations for PVC %s", persistentVolumeClaim.Name) - if err = setPvcAnnotations(client, persistentVolumeClaim, annotations); err != nil { + if err := setPvcAnnotations(client, persistentVolumeClaim, annotations); err != nil { return err } } + + return nil + }, func(persistentVolumeClaim *core.PersistentVolumeClaim) bool { + return k8sutil.IsChildResource(kind, name, namespace, persistentVolumeClaim) + }); err != nil { + return err } return nil @@ -297,25 +297,23 @@ func getPodGroup(pod *core.Pod) api.ServerGroup { return api.ServerGroupFromRole(pod.Labels[k8sutil.LabelKeyRole]) } -func ensurePodsAnnotations(client typedCore.PodInterface, kind, name, namespace string, annotations map[string]string, spec api.DeploymentSpec) error { - pods, err := k8sutil.GetPodsForParent(client, - kind, - name, - namespace) - if err != nil { - return err - } - - for _, pod := range pods { +func ensurePodsAnnotations(client typedCore.PodInterface, cachedStatus inspector.Inspector, kind, name, namespace string, annotations map[string]string, spec api.DeploymentSpec) error { + if err := cachedStatus.IteratePods(func(pod *core.Pod) error { group := getPodGroup(pod) mergedAnnotations := k8sutil.MergeAnnotations(annotations, spec.GetServerGroupSpec(group).Annotations) if !k8sutil.CompareAnnotations(pod.GetAnnotations(), mergedAnnotations) { log.Info().Msgf("Replacing annotations for Pod %s", pod.Name) - if err = setPodAnnotations(client, pod, mergedAnnotations); err != nil { + if err := setPodAnnotations(client, pod, mergedAnnotations); err != nil { return err } } + + return nil + }, func(pod *core.Pod) bool { + return k8sutil.IsChildResource(kind, name, namespace, pod) + }); err != nil { + return err } return nil diff --git a/pkg/deployment/resources/context.go b/pkg/deployment/resources/context.go index 519a8250d..6f353e4fb 100644 --- a/pkg/deployment/resources/context.go +++ b/pkg/deployment/resources/context.go @@ -74,13 +74,11 @@ type Context interface { // CreateEvent creates a given event. // On error, the error is logged. CreateEvent(evt *k8sutil.Event) - // GetOwnedPods returns a list of all pods owned by the deployment. - GetOwnedPods() ([]v1.Pod, error) // GetOwnedPVCs returns a list of all PVCs owned by the deployment. GetOwnedPVCs() ([]v1.PersistentVolumeClaim, error) // CleanupPod deletes a given pod with force and explicit UID. // If the pod does not exist, the error is ignored. - CleanupPod(p v1.Pod) error + CleanupPod(p *v1.Pod) error // DeletePod deletes a pod with given name in the namespace // of the deployment. If the pod does not exist, the error is ignored. DeletePod(podName string) error diff --git a/pkg/deployment/resources/inspector/inspector.go b/pkg/deployment/resources/inspector/inspector.go new file mode 100644 index 000000000..189e579cb --- /dev/null +++ b/pkg/deployment/resources/inspector/inspector.go @@ -0,0 +1,86 @@ +// +// DISCLAIMER +// +// Copyright 2020 ArangoDB GmbH, Cologne, Germany +// +// 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. +// +// Copyright holder is ArangoDB GmbH, Cologne, Germany +// +// Author Adam Janikowski +// + +package inspector + +import ( + core "k8s.io/api/core/v1" + "k8s.io/client-go/kubernetes" +) + +func NewInspector(k kubernetes.Interface, namespace string) (Inspector, error) { + pods, err := podsToMap(k, namespace) + if err != nil { + return nil, err + } + + secrets, err := secretsToMap(k, namespace) + if err != nil { + return nil, err + } + + pvcs, err := pvcsToMap(k, namespace) + if err != nil { + return nil, err + } + + services, err := servicesToMap(k, namespace) + if err != nil { + return nil, err + } + + return NewInspectorFromData(pods, secrets, pvcs, services), nil +} + +func NewEmptyInspector() Inspector { + return NewInspectorFromData(nil, nil, nil, nil) +} + +func NewInspectorFromData(pods map[string]*core.Pod, secrets map[string]*core.Secret, pvcs map[string]*core.PersistentVolumeClaim, services map[string]*core.Service) Inspector { + return &inspector{ + pods: pods, + secrets: secrets, + pvcs: pvcs, + services: services, + } +} + +type Inspector interface { + Pod(name string) (*core.Pod, bool) + IteratePods(action PodAction, filters ...PodFilter) error + + Secret(name string) (*core.Secret, bool) + IterateSecrets(action SecretAction, filters ...SecretFilter) error + + PersistentVolumeClaim(name string) (*core.PersistentVolumeClaim, bool) + IteratePersistentVolumeClaims(action PersistentVolumeClaimAction, filters ...PersistentVolumeClaimFilter) error + + Service(name string) (*core.Service, bool) + IterateServices(action ServiceAction, filters ...ServiceFilter) error +} + +type inspector struct { + pods map[string]*core.Pod + secrets map[string]*core.Secret + pvcs map[string]*core.PersistentVolumeClaim + services map[string]*core.Service +} diff --git a/pkg/deployment/resources/inspector/pods.go b/pkg/deployment/resources/inspector/pods.go new file mode 100644 index 000000000..70a5bcd6e --- /dev/null +++ b/pkg/deployment/resources/inspector/pods.go @@ -0,0 +1,124 @@ +// +// DISCLAIMER +// +// Copyright 2020 ArangoDB GmbH, Cologne, Germany +// +// 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. +// +// Copyright holder is ArangoDB GmbH, Cologne, Germany +// +// Author Adam Janikowski +// + +package inspector + +import ( + "github.com/pkg/errors" + core "k8s.io/api/core/v1" + meta "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" +) + +type PodFilter func(pod *core.Pod) bool +type PodAction func(pod *core.Pod) error + +func (i *inspector) IteratePods(action PodAction, filters ...PodFilter) error { + for _, pod := range i.pods { + if err := i.iteratePod(pod, action, filters...); err != nil { + return err + } + } + return nil +} + +func (i *inspector) iteratePod(pod *core.Pod, action PodAction, filters ...PodFilter) error { + for _, filter := range filters { + if !filter(pod) { + return nil + } + } + + return action(pod) +} + +func (i *inspector) Pod(name string) (*core.Pod, bool) { + pod, ok := i.pods[name] + if !ok { + return nil, false + } + + return pod, true +} + +func podsToMap(k kubernetes.Interface, namespace string) (map[string]*core.Pod, error) { + pods, err := getPods(k, namespace, "") + if err != nil { + return nil, err + } + + podMap := map[string]*core.Pod{} + + for _, pod := range pods { + _, exists := podMap[pod.GetName()] + if exists { + return nil, errors.Errorf("Pod %s already exists in map, error received", pod.GetName()) + } + + podMap[pod.GetName()] = podPointer(pod) + } + + return podMap, nil +} + +func podPointer(pod core.Pod) *core.Pod { + return &pod +} + +func getPods(k kubernetes.Interface, namespace, cont string) ([]core.Pod, error) { + pods, err := k.CoreV1().Pods(namespace).List(meta.ListOptions{ + Limit: 128, + Continue: cont, + }) + + if err != nil { + return nil, err + } + + if pods.Continue != "" { + nextPodsLayer, err := getPods(k, namespace, pods.Continue) + if err != nil { + return nil, err + } + + return append(pods.Items, nextPodsLayer...), nil + } + + return pods.Items, nil +} + +func FilterPodsByLabels(labels map[string]string) PodFilter { + return func(pod *core.Pod) bool { + for key, value := range labels { + v, ok := pod.Labels[key] + if !ok { + return false + } + + if v != value { + return false + } + } + + return true + } +} diff --git a/pkg/deployment/resources/inspector/pvcs.go b/pkg/deployment/resources/inspector/pvcs.go new file mode 100644 index 000000000..1f97e004f --- /dev/null +++ b/pkg/deployment/resources/inspector/pvcs.go @@ -0,0 +1,124 @@ +// +// DISCLAIMER +// +// Copyright 2020 ArangoDB GmbH, Cologne, Germany +// +// 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. +// +// Copyright holder is ArangoDB GmbH, Cologne, Germany +// +// Author Adam Janikowski +// + +package inspector + +import ( + "github.com/pkg/errors" + core "k8s.io/api/core/v1" + meta "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" +) + +type PersistentVolumeClaimFilter func(pvc *core.PersistentVolumeClaim) bool +type PersistentVolumeClaimAction func(pvc *core.PersistentVolumeClaim) error + +func (i *inspector) IteratePersistentVolumeClaims(action PersistentVolumeClaimAction, filters ...PersistentVolumeClaimFilter) error { + for _, pvc := range i.pvcs { + if err := i.iteratePersistentVolumeClaim(pvc, action, filters...); err != nil { + return err + } + } + return nil +} + +func (i *inspector) iteratePersistentVolumeClaim(pvc *core.PersistentVolumeClaim, action PersistentVolumeClaimAction, filters ...PersistentVolumeClaimFilter) error { + for _, filter := range filters { + if !filter(pvc) { + return nil + } + } + + return action(pvc) +} + +func (i *inspector) PersistentVolumeClaim(name string) (*core.PersistentVolumeClaim, bool) { + pvc, ok := i.pvcs[name] + if !ok { + return nil, false + } + + return pvc, true +} + +func pvcsToMap(k kubernetes.Interface, namespace string) (map[string]*core.PersistentVolumeClaim, error) { + pvcs, err := getPersistentVolumeClaims(k, namespace, "") + if err != nil { + return nil, err + } + + pvcMap := map[string]*core.PersistentVolumeClaim{} + + for _, pvc := range pvcs { + _, exists := pvcMap[pvc.GetName()] + if exists { + return nil, errors.Errorf("PersistentVolumeClaim %s already exists in map, error received", pvc.GetName()) + } + + pvcMap[pvc.GetName()] = pvcPointer(pvc) + } + + return pvcMap, nil +} + +func pvcPointer(pvc core.PersistentVolumeClaim) *core.PersistentVolumeClaim { + return &pvc +} + +func getPersistentVolumeClaims(k kubernetes.Interface, namespace, cont string) ([]core.PersistentVolumeClaim, error) { + pvcs, err := k.CoreV1().PersistentVolumeClaims(namespace).List(meta.ListOptions{ + Limit: 128, + Continue: cont, + }) + + if err != nil { + return nil, err + } + + if pvcs.Continue != "" { + nextPersistentVolumeClaimsLayer, err := getPersistentVolumeClaims(k, namespace, pvcs.Continue) + if err != nil { + return nil, err + } + + return append(pvcs.Items, nextPersistentVolumeClaimsLayer...), nil + } + + return pvcs.Items, nil +} + +func FilterPersistentVolumeClaimsByLabels(labels map[string]string) PersistentVolumeClaimFilter { + return func(pvc *core.PersistentVolumeClaim) bool { + for key, value := range labels { + v, ok := pvc.Labels[key] + if !ok { + return false + } + + if v != value { + return false + } + } + + return true + } +} diff --git a/pkg/deployment/resources/inspector/secrets.go b/pkg/deployment/resources/inspector/secrets.go new file mode 100644 index 000000000..913562028 --- /dev/null +++ b/pkg/deployment/resources/inspector/secrets.go @@ -0,0 +1,107 @@ +// +// DISCLAIMER +// +// Copyright 2020 ArangoDB GmbH, Cologne, Germany +// +// 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. +// +// Copyright holder is ArangoDB GmbH, Cologne, Germany +// +// Author Adam Janikowski +// + +package inspector + +import ( + "github.com/pkg/errors" + core "k8s.io/api/core/v1" + meta "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" +) + +type SecretFilter func(pod *core.Secret) bool +type SecretAction func(pod *core.Secret) error + +func (i *inspector) IterateSecrets(action SecretAction, filters ...SecretFilter) error { + for _, secret := range i.secrets { + if err := i.iterateSecrets(secret, action, filters...); err != nil { + return err + } + } + return nil +} + +func (i *inspector) iterateSecrets(secret *core.Secret, action SecretAction, filters ...SecretFilter) error { + for _, filter := range filters { + if !filter(secret) { + return nil + } + } + + return action(secret) +} + +func (i *inspector) Secret(name string) (*core.Secret, bool) { + secret, ok := i.secrets[name] + if !ok { + return nil, false + } + + return secret, true +} + +func secretsToMap(k kubernetes.Interface, namespace string) (map[string]*core.Secret, error) { + secrets, err := getSecrets(k, namespace, "") + if err != nil { + return nil, err + } + + secretMap := map[string]*core.Secret{} + + for _, secret := range secrets { + _, exists := secretMap[secret.GetName()] + if exists { + return nil, errors.Errorf("Secret %s already exists in map, error received", secret.GetName()) + } + + secretMap[secret.GetName()] = secretPointer(secret) + } + + return secretMap, nil +} + +func secretPointer(pod core.Secret) *core.Secret { + return &pod +} + +func getSecrets(k kubernetes.Interface, namespace, cont string) ([]core.Secret, error) { + secrets, err := k.CoreV1().Secrets(namespace).List(meta.ListOptions{ + Limit: 128, + Continue: cont, + }) + + if err != nil { + return nil, err + } + + if secrets.Continue != "" { + nextSecretsLayer, err := getSecrets(k, namespace, secrets.Continue) + if err != nil { + return nil, err + } + + return append(secrets.Items, nextSecretsLayer...), nil + } + + return secrets.Items, nil +} diff --git a/pkg/deployment/resources/inspector/services.go b/pkg/deployment/resources/inspector/services.go new file mode 100644 index 000000000..1dccd5171 --- /dev/null +++ b/pkg/deployment/resources/inspector/services.go @@ -0,0 +1,107 @@ +// +// DISCLAIMER +// +// Copyright 2020 ArangoDB GmbH, Cologne, Germany +// +// 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. +// +// Copyright holder is ArangoDB GmbH, Cologne, Germany +// +// Author Adam Janikowski +// + +package inspector + +import ( + "github.com/pkg/errors" + core "k8s.io/api/core/v1" + meta "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" +) + +type ServiceFilter func(pod *core.Service) bool +type ServiceAction func(pod *core.Service) error + +func (i *inspector) IterateServices(action ServiceAction, filters ...ServiceFilter) error { + for _, service := range i.services { + if err := i.iterateServices(service, action, filters...); err != nil { + return err + } + } + return nil +} + +func (i *inspector) iterateServices(service *core.Service, action ServiceAction, filters ...ServiceFilter) error { + for _, filter := range filters { + if !filter(service) { + return nil + } + } + + return action(service) +} + +func (i *inspector) Service(name string) (*core.Service, bool) { + service, ok := i.services[name] + if !ok { + return nil, false + } + + return service, true +} + +func servicesToMap(k kubernetes.Interface, namespace string) (map[string]*core.Service, error) { + services, err := getServices(k, namespace, "") + if err != nil { + return nil, err + } + + serviceMap := map[string]*core.Service{} + + for _, service := range services { + _, exists := serviceMap[service.GetName()] + if exists { + return nil, errors.Errorf("Service %s already exists in map, error received", service.GetName()) + } + + serviceMap[service.GetName()] = servicePointer(service) + } + + return serviceMap, nil +} + +func servicePointer(pod core.Service) *core.Service { + return &pod +} + +func getServices(k kubernetes.Interface, namespace, cont string) ([]core.Service, error) { + services, err := k.CoreV1().Services(namespace).List(meta.ListOptions{ + Limit: 128, + Continue: cont, + }) + + if err != nil { + return nil, err + } + + if services.Continue != "" { + nextServicesLayer, err := getServices(k, namespace, services.Continue) + if err != nil { + return nil, err + } + + return append(services.Items, nextServicesLayer...), nil + } + + return services.Items, nil +} diff --git a/pkg/deployment/resources/license.go b/pkg/deployment/resources/license.go index 72035971d..22bebc23b 100644 --- a/pkg/deployment/resources/license.go +++ b/pkg/deployment/resources/license.go @@ -21,29 +21,27 @@ package resources import ( - "fmt" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + "github.com/pkg/errors" "github.com/arangodb/kube-arangodb/pkg/util/constants" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // ValidateLicenseKeySecret checks if the licens key secret exists and is valid -func (r *Resources) ValidateLicenseKeySecret() error { +func (r *Resources) ValidateLicenseKeySecret(cachedStatus inspector.Inspector) error { spec := r.context.GetSpec().License if spec.HasSecretName() { secretName := spec.GetSecretName() - kubecli := r.context.GetKubeCli() - ns := r.context.GetNamespace() - s, err := kubecli.CoreV1().Secrets(ns).Get(secretName, metav1.GetOptions{}) + s, exists := cachedStatus.Secret(secretName) - if err != nil { - return err + if !exists { + return errors.Errorf("License secret %s does not exist", s) } if _, ok := s.Data[constants.SecretKeyToken]; !ok { - return fmt.Errorf("Invalid secret format") + return errors.Errorf("Invalid secret format") } } diff --git a/pkg/deployment/resources/pod_cleanup.go b/pkg/deployment/resources/pod_cleanup.go index 4f745276b..7f5786c79 100644 --- a/pkg/deployment/resources/pod_cleanup.go +++ b/pkg/deployment/resources/pod_cleanup.go @@ -25,6 +25,9 @@ package resources import ( "time" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + v1 "k8s.io/api/core/v1" + "github.com/arangodb/kube-arangodb/pkg/util" "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" @@ -38,58 +41,57 @@ const ( // CleanupTerminatedPods removes all pods in Terminated state that belong to a member in Created state. // Returns: Interval_till_next_inspection, error -func (r *Resources) CleanupTerminatedPods() (util.Interval, error) { +func (r *Resources) CleanupTerminatedPods(cachedStatus inspector.Inspector) (util.Interval, error) { log := r.log nextInterval := maxPodInspectorInterval // Large by default, will be made smaller if needed in the rest of the function - pods, err := r.context.GetOwnedPods() - if err != nil { - log.Debug().Err(err).Msg("Failed to get owned pods") - return 0, maskAny(err) - } - // Update member status from all pods found status, _ := r.context.GetStatus() - for _, p := range pods { - if k8sutil.IsArangoDBImageIDAndVersionPod(p) { + err := cachedStatus.IteratePods(func(pod *v1.Pod) error { + if k8sutil.IsArangoDBImageIDAndVersionPod(pod) { // Image ID pods are not relevant to inspect here - continue + return nil } - // Check pod state - if !(k8sutil.IsPodSucceeded(&p) || k8sutil.IsPodFailed(&p) || k8sutil.IsPodTerminating(&p)) { - continue + if !(k8sutil.IsPodSucceeded(pod) || k8sutil.IsPodFailed(pod) || k8sutil.IsPodTerminating(pod)) { + return nil } // Find member status - memberStatus, group, found := status.Members.MemberStatusByPodName(p.GetName()) + memberStatus, group, found := status.Members.MemberStatusByPodName(pod.GetName()) if !found { - log.Debug().Str("pod", p.GetName()).Msg("no memberstatus found for pod. Performing cleanup") + log.Debug().Str("pod", pod.GetName()).Msg("no memberstatus found for pod. Performing cleanup") } else { // Check member termination condition if !memberStatus.Conditions.IsTrue(api.ConditionTypeTerminated) { if !group.IsStateless() { // For statefull members, we have to wait for confirmed termination - log.Debug().Str("pod", p.GetName()).Msg("Cannot cleanup pod yet, waiting for it to reach terminated state") + log.Debug().Str("pod", pod.GetName()).Msg("Cannot cleanup pod yet, waiting for it to reach terminated state") nextInterval = nextInterval.ReduceTo(recheckStatefullPodCleanupInterval) - continue + return nil } else { // If a stateless server does not terminate within a reasonable amount or time, we kill it. - t := p.GetDeletionTimestamp() + t := pod.GetDeletionTimestamp() if t == nil || t.Add(statelessTerminationPeriod).After(time.Now()) { // Either delete timestamp is not set, or not yet waiting long enough nextInterval = nextInterval.ReduceTo(util.Interval(statelessTerminationPeriod)) - continue + return nil } } } } // Ok, we can delete the pod - log.Debug().Str("pod-name", p.GetName()).Msg("Cleanup terminated pod") - if err := r.context.CleanupPod(p); err != nil { - log.Warn().Err(err).Str("pod-name", p.GetName()).Msg("Failed to cleanup pod") + log.Debug().Str("pod-name", pod.GetName()).Msg("Cleanup terminated pod") + if err := r.context.CleanupPod(pod); err != nil { + log.Warn().Err(err).Str("pod-name", pod.GetName()).Msg("Failed to cleanup pod") } + + return nil + }, inspector.FilterPodsByLabels(k8sutil.LabelsForDeployment(r.context.GetAPIObject().GetName(), ""))) + if err != nil { + return 0, err } + return nextInterval, nil } diff --git a/pkg/deployment/resources/pod_creator.go b/pkg/deployment/resources/pod_creator.go index b04de11c7..a84a4b72c 100644 --- a/pkg/deployment/resources/pod_creator.go +++ b/pkg/deployment/resources/pod_creator.go @@ -33,6 +33,9 @@ import ( "sync" "time" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/interfaces" + "k8s.io/apimachinery/pkg/types" "github.com/arangodb/kube-arangodb/pkg/deployment/pod" @@ -298,7 +301,7 @@ func (r *Resources) CreatePodTolerations(group api.ServerGroup, groupSpec api.Se return tolerations } -func (r *Resources) RenderPodForMember(spec api.DeploymentSpec, status api.DeploymentStatus, memberID string, imageInfo api.ImageInfo) (*core.Pod, error) { +func (r *Resources) RenderPodForMember(cachedStatus inspector.Inspector, spec api.DeploymentSpec, status api.DeploymentStatus, memberID string, imageInfo api.ImageInfo) (*core.Pod, error) { log := r.log apiObject := r.context.GetAPIObject() m, group, found := status.Members.ElementByID(memberID) @@ -360,24 +363,10 @@ func (r *Resources) RenderPodForMember(spec api.DeploymentSpec, status api.Deplo args := createArangodArgs(input) - if err := memberPod.Validate(secrets); err != nil { + if err := memberPod.Validate(cachedStatus); err != nil { return nil, maskAny(errors.Wrapf(err, "Validation of pods resources failed")) } - if rocksdbEncryptionSecretName != "" { - if err := k8sutil.ValidateEncryptionKeySecret(secrets, rocksdbEncryptionSecretName); err != nil { - return nil, maskAny(errors.Wrapf(err, "RocksDB encryption key secret validation failed")) - } - } - - // Check cluster JWT secret - if clusterJWTSecretName != "" { - if err := k8sutil.ValidateTokenSecret(secrets, clusterJWTSecretName); err != nil { - return nil, maskAny(errors.Wrapf(err, "Cluster JWT secret validation failed")) - } - - } - return RenderArangoPod(apiObject, role, m.ID, m.PodName, args, &memberPod) } else if group.IsArangosync() { // Check image @@ -461,7 +450,7 @@ func (r *Resources) SelectImage(spec api.DeploymentSpec, status api.DeploymentSt } // createPodForMember creates all Pods listed in member status -func (r *Resources) createPodForMember(spec api.DeploymentSpec, memberID string, imageNotFoundOnce *sync.Once) error { +func (r *Resources) createPodForMember(spec api.DeploymentSpec, memberID string, imageNotFoundOnce *sync.Once, cachedStatus inspector.Inspector) error { log := r.log status, lastVersion := r.context.GetStatus() @@ -475,7 +464,7 @@ func (r *Resources) createPodForMember(spec api.DeploymentSpec, memberID string, } status.CurrentImage = &imageInfo - pod, err := r.RenderPodForMember(spec, status, memberID, imageInfo) + pod, err := r.RenderPodForMember(cachedStatus, spec, status, memberID, imageInfo) if err != nil { return maskAny(err) } @@ -587,7 +576,7 @@ func (r *Resources) createPodForMember(spec api.DeploymentSpec, memberID string, // RenderArangoPod renders new ArangoD Pod func RenderArangoPod(deployment k8sutil.APIObject, role, id, podName string, - args []string, podCreator k8sutil.PodCreator) (*core.Pod, error) { + args []string, podCreator interfaces.PodCreator) (*core.Pod, error) { // Prepare basic pod p := k8sutil.NewPod(deployment.GetName(), role, id, podName, podCreator) @@ -631,7 +620,7 @@ func CreateArangoPod(kubecli kubernetes.Interface, deployment k8sutil.APIObject, } // EnsurePods creates all Pods listed in member status -func (r *Resources) EnsurePods() error { +func (r *Resources) EnsurePods(cachedStatus inspector.Inspector) error { iterator := r.context.GetServerGroupIterator() deploymentStatus, _ := r.context.GetStatus() imageNotFoundOnce := &sync.Once{} @@ -645,7 +634,7 @@ func (r *Resources) EnsurePods() error { continue } spec := r.context.GetSpec() - if err := r.createPodForMember(spec, m.ID, imageNotFoundOnce); err != nil { + if err := r.createPodForMember(spec, m.ID, imageNotFoundOnce, cachedStatus); err != nil { return maskAny(err) } } diff --git a/pkg/deployment/resources/pod_creator_arangod.go b/pkg/deployment/resources/pod_creator_arangod.go index ab1598e89..011a16172 100644 --- a/pkg/deployment/resources/pod_creator_arangod.go +++ b/pkg/deployment/resources/pod_creator_arangod.go @@ -27,6 +27,9 @@ import ( "math" "os" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/interfaces" + "github.com/arangodb/kube-arangodb/pkg/deployment/pod" "github.com/arangodb/kube-arangodb/pkg/util" @@ -43,8 +46,8 @@ const ( ArangoDBOverrideDetectedTotalMemoryEnv = "ARANGODB_OVERRIDE_DETECTED_TOTAL_MEMORY" ) -var _ k8sutil.PodCreator = &MemberArangoDPod{} -var _ k8sutil.ContainerCreator = &ArangoDContainer{} +var _ interfaces.PodCreator = &MemberArangoDPod{} +var _ interfaces.ContainerCreator = &ArangoDContainer{} type MemberArangoDPod struct { status api.MemberStatus @@ -207,9 +210,17 @@ func (m *MemberArangoDPod) Init(pod *core.Pod) { pod.Spec.PriorityClassName = m.groupSpec.PriorityClassName } -func (m *MemberArangoDPod) Validate(secrets k8sutil.SecretInterface) error { +func (m *MemberArangoDPod) Validate(cachedStatus inspector.Inspector) error { i := m.AsInput() - if err := pod.SNI().Verify(i, secrets); err != nil { + if err := pod.SNI().Verify(i, cachedStatus); err != nil { + return err + } + + if err := pod.Encryption().Verify(i, cachedStatus); err != nil { + return err + } + + if err := pod.JWT().Verify(i, cachedStatus); err != nil { return err } @@ -428,7 +439,7 @@ func (m *MemberArangoDPod) GetTolerations() []core.Toleration { return m.resources.CreatePodTolerations(m.group, m.groupSpec) } -func (m *MemberArangoDPod) GetContainerCreator() k8sutil.ContainerCreator { +func (m *MemberArangoDPod) GetContainerCreator() interfaces.ContainerCreator { return &ArangoDContainer{ spec: m.spec, group: m.group, diff --git a/pkg/deployment/resources/pod_creator_sync.go b/pkg/deployment/resources/pod_creator_sync.go index 096d60258..6675c1e6c 100644 --- a/pkg/deployment/resources/pod_creator_sync.go +++ b/pkg/deployment/resources/pod_creator_sync.go @@ -25,6 +25,9 @@ package resources import ( "math" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/interfaces" + "github.com/arangodb/kube-arangodb/pkg/deployment/pod" "github.com/arangodb/kube-arangodb/pkg/util/constants" @@ -46,8 +49,8 @@ type ArangoSyncContainer struct { imageInfo api.ImageInfo } -var _ k8sutil.PodCreator = &MemberSyncPod{} -var _ k8sutil.ContainerCreator = &ArangoSyncContainer{} +var _ interfaces.PodCreator = &MemberSyncPod{} +var _ interfaces.ContainerCreator = &ArangoSyncContainer{} type MemberSyncPod struct { tlsKeyfileSecretName string @@ -277,7 +280,7 @@ func (m *MemberSyncPod) GetTolerations() []core.Toleration { return m.resources.CreatePodTolerations(m.group, m.groupSpec) } -func (m *MemberSyncPod) GetContainerCreator() k8sutil.ContainerCreator { +func (m *MemberSyncPod) GetContainerCreator() interfaces.ContainerCreator { return &ArangoSyncContainer{ groupSpec: m.groupSpec, spec: m.spec, @@ -293,6 +296,6 @@ func (m *MemberSyncPod) Init(pod *core.Pod) { pod.Spec.PriorityClassName = m.groupSpec.PriorityClassName } -func (m *MemberSyncPod) Validate(secrets k8sutil.SecretInterface) error { +func (m *MemberSyncPod) Validate(cachedStatus inspector.Inspector) error { return nil } diff --git a/pkg/deployment/resources/pod_inspector.go b/pkg/deployment/resources/pod_inspector.go index e7daacb52..241537125 100644 --- a/pkg/deployment/resources/pod_inspector.go +++ b/pkg/deployment/resources/pod_inspector.go @@ -27,6 +27,9 @@ import ( "fmt" "time" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" @@ -49,7 +52,7 @@ const ( // InspectPods lists all pods that belong to the given deployment and updates // the member status of the deployment accordingly. // Returns: Interval_till_next_inspection, error -func (r *Resources) InspectPods(ctx context.Context) (util.Interval, error) { +func (r *Resources) InspectPods(ctx context.Context, cachedStatus inspector.Inspector) (util.Interval, error) { log := r.log start := time.Now() apiObject := r.context.GetAPIObject() @@ -58,50 +61,43 @@ func (r *Resources) InspectPods(ctx context.Context) (util.Interval, error) { nextInterval := maxPodInspectorInterval // Large by default, will be made smaller if needed in the rest of the function defer metrics.SetDuration(inspectPodsDurationGauges.WithLabelValues(deploymentName), start) - pods, err := r.context.GetOwnedPods() - if err != nil { - log.Debug().Err(err).Msg("Failed to get owned pods") - return 0, maskAny(err) - } - - // Update member status from all pods found status, lastVersion := r.context.GetStatus() var podNamesWithScheduleTimeout []string var unscheduledPodNames []string - for _, p := range pods { - if k8sutil.IsArangoDBImageIDAndVersionPod(p) { + + err := cachedStatus.IteratePods(func(pod *v1.Pod) error { + if k8sutil.IsArangoDBImageIDAndVersionPod(pod) { // Image ID pods are not relevant to inspect here - continue + return nil } // Pod belongs to this deployment, update metric inspectedPodsCounters.WithLabelValues(deploymentName).Inc() - // Find member status - memberStatus, group, found := status.Members.MemberStatusByPodName(p.GetName()) + memberStatus, group, found := status.Members.MemberStatusByPodName(pod.GetName()) if !found { - log.Debug().Str("pod", p.GetName()).Msg("no memberstatus found for pod") - if k8sutil.IsPodMarkedForDeletion(&p) && len(p.GetFinalizers()) > 0 { + log.Debug().Str("pod", pod.GetName()).Msg("no memberstatus found for pod") + if k8sutil.IsPodMarkedForDeletion(pod) && len(pod.GetFinalizers()) > 0 { // Strange, pod belongs to us, but we have no member for it. // Remove all finalizers, so it can be removed. log.Warn().Msg("Pod belongs to this deployment, but we don't know the member. Removing all finalizers") kubecli := r.context.GetKubeCli() ignoreNotFound := false - if err := k8sutil.RemovePodFinalizers(log, kubecli, &p, p.GetFinalizers(), ignoreNotFound); err != nil { + if err := k8sutil.RemovePodFinalizers(log, kubecli, pod, pod.GetFinalizers(), ignoreNotFound); err != nil { log.Debug().Err(err).Msg("Failed to update pod (to remove all finalizers)") - return 0, maskAny(err) + return maskAny(err) } } - continue + return nil } // Update state updateMemberStatusNeeded := false - if k8sutil.IsPodSucceeded(&p) { + if k8sutil.IsPodSucceeded(pod) { // Pod has terminated with exit code 0. wasTerminated := memberStatus.Conditions.IsTrue(api.ConditionTypeTerminated) if memberStatus.Conditions.Update(api.ConditionTypeTerminated, true, "Pod Succeeded", "") { - log.Debug().Str("pod-name", p.GetName()).Msg("Updating member condition Terminated to true: Pod Succeeded") + log.Debug().Str("pod-name", pod.GetName()).Msg("Updating member condition Terminated to true: Pod Succeeded") updateMemberStatusNeeded = true nextInterval = nextInterval.ReduceTo(recheckSoonPodInspectorInterval) if !wasTerminated { @@ -111,11 +107,11 @@ func (r *Resources) InspectPods(ctx context.Context) (util.Interval, error) { r.InvalidateSyncStatus() } } - } else if k8sutil.IsPodFailed(&p) { + } else if k8sutil.IsPodFailed(pod) { // Pod has terminated with at least 1 container with a non-zero exit code. wasTerminated := memberStatus.Conditions.IsTrue(api.ConditionTypeTerminated) if memberStatus.Conditions.Update(api.ConditionTypeTerminated, true, "Pod Failed", "") { - log.Debug().Str("pod-name", p.GetName()).Msg("Updating member condition Terminated to true: Pod Failed") + log.Debug().Str("pod-name", pod.GetName()).Msg("Updating member condition Terminated to true: Pod Failed") updateMemberStatusNeeded = true nextInterval = nextInterval.ReduceTo(recheckSoonPodInspectorInterval) if !wasTerminated { @@ -126,10 +122,11 @@ func (r *Resources) InspectPods(ctx context.Context) (util.Interval, error) { } } } - if k8sutil.IsPodReady(&p) { + + if k8sutil.IsPodReady(pod) { // Pod is now ready if memberStatus.Conditions.Update(api.ConditionTypeReady, true, "Pod Ready", "") { - log.Debug().Str("pod-name", p.GetName()).Msg("Updating member condition Ready to true") + log.Debug().Str("pod-name", pod.GetName()).Msg("Updating member condition Ready to true") memberStatus.IsInitialized = true // Require future pods for this member to have an existing UUID (in case of dbserver). updateMemberStatusNeeded = true nextInterval = nextInterval.ReduceTo(recheckSoonPodInspectorInterval) @@ -137,25 +134,27 @@ func (r *Resources) InspectPods(ctx context.Context) (util.Interval, error) { } else { // Pod is not ready if memberStatus.Conditions.Update(api.ConditionTypeReady, false, "Pod Not Ready", "") { - log.Debug().Str("pod-name", p.GetName()).Msg("Updating member condition Ready to false") + log.Debug().Str("pod-name", pod.GetName()).Msg("Updating member condition Ready to false") updateMemberStatusNeeded = true nextInterval = nextInterval.ReduceTo(recheckSoonPodInspectorInterval) } } - if k8sutil.IsPodNotScheduledFor(&p, podScheduleTimeout) { + + if k8sutil.IsPodNotScheduledFor(pod, podScheduleTimeout) { // Pod cannot be scheduled for to long - log.Debug().Str("pod-name", p.GetName()).Msg("Pod scheduling timeout") - podNamesWithScheduleTimeout = append(podNamesWithScheduleTimeout, p.GetName()) - } else if !k8sutil.IsPodScheduled(&p) { - unscheduledPodNames = append(unscheduledPodNames, p.GetName()) + log.Debug().Str("pod-name", pod.GetName()).Msg("Pod scheduling timeout") + podNamesWithScheduleTimeout = append(podNamesWithScheduleTimeout, pod.GetName()) + } else if !k8sutil.IsPodScheduled(pod) { + unscheduledPodNames = append(unscheduledPodNames, pod.GetName()) } - if k8sutil.IsPodMarkedForDeletion(&p) { + + if k8sutil.IsPodMarkedForDeletion(pod) { if memberStatus.Conditions.Update(api.ConditionTypeTerminating, true, "Pod marked for deletion", "") { updateMemberStatusNeeded = true - log.Debug().Str("pod-name", p.GetName()).Msg("Pod marked as terminating") + log.Debug().Str("pod-name", pod.GetName()).Msg("Pod marked as terminating") } // Process finalizers - if x, err := r.runPodFinalizers(ctx, &p, memberStatus, func(m api.MemberStatus) error { + if x, err := r.runPodFinalizers(ctx, pod, memberStatus, func(m api.MemberStatus) error { updateMemberStatusNeeded = true memberStatus = m return nil @@ -166,27 +165,24 @@ func (r *Resources) InspectPods(ctx context.Context) (util.Interval, error) { nextInterval = nextInterval.ReduceTo(x) } } + if updateMemberStatusNeeded { if err := status.Members.Update(memberStatus, group); err != nil { - return 0, maskAny(err) + return maskAny(err) } } - } - podExists := func(podName string) bool { - for _, p := range pods { - if p.GetName() == podName { - return true - } - } - return false + return nil + }, inspector.FilterPodsByLabels(k8sutil.LabelsForDeployment(deploymentName, ""))) + if err != nil { + return 0, err } // Go over all members, check for missing pods status.Members.ForeachServerGroup(func(group api.ServerGroup, members api.MemberStatusList) error { for _, m := range members { if podName := m.PodName; podName != "" { - if !podExists(podName) { + if _, exists := cachedStatus.Pod(podName); !exists { log.Debug().Str("pod-name", podName).Msg("Does not exist") switch m.Phase { case api.MemberPhaseNone: diff --git a/pkg/deployment/resources/pvc_inspector.go b/pkg/deployment/resources/pvc_inspector.go index 0e2e84c69..7a242a421 100644 --- a/pkg/deployment/resources/pvc_inspector.go +++ b/pkg/deployment/resources/pvc_inspector.go @@ -26,6 +26,9 @@ import ( "context" "time" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + v1 "k8s.io/api/core/v1" + "github.com/arangodb/kube-arangodb/pkg/metrics" "github.com/arangodb/kube-arangodb/pkg/util" "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" @@ -42,52 +45,50 @@ const ( // InspectPVCs lists all PVCs that belong to the given deployment and updates // the member status of the deployment accordingly. -func (r *Resources) InspectPVCs(ctx context.Context) (util.Interval, error) { +func (r *Resources) InspectPVCs(ctx context.Context, cachedStatus inspector.Inspector) (util.Interval, error) { log := r.log start := time.Now() nextInterval := maxPVCInspectorInterval deploymentName := r.context.GetAPIObject().GetName() defer metrics.SetDuration(inspectPVCsDurationGauges.WithLabelValues(deploymentName), start) - pvcs, err := r.context.GetOwnedPVCs() - if err != nil { - log.Debug().Err(err).Msg("Failed to get owned PVCs") - return 0, maskAny(err) - } - // Update member status from all pods found status, _ := r.context.GetStatus() - for _, p := range pvcs { + if err := cachedStatus.IteratePersistentVolumeClaims(func(pvc *v1.PersistentVolumeClaim) error { // PVC belongs to this deployment, update metric inspectedPVCsCounters.WithLabelValues(deploymentName).Inc() // Find member status - memberStatus, group, found := status.Members.MemberStatusByPVCName(p.GetName()) + memberStatus, group, found := status.Members.MemberStatusByPVCName(pvc.GetName()) if !found { - log.Debug().Str("pvc", p.GetName()).Msg("no memberstatus found for PVC") - if k8sutil.IsPersistentVolumeClaimMarkedForDeletion(&p) && len(p.GetFinalizers()) > 0 { + log.Debug().Str("pvc", pvc.GetName()).Msg("no memberstatus found for PVC") + if k8sutil.IsPersistentVolumeClaimMarkedForDeletion(pvc) && len(pvc.GetFinalizers()) > 0 { // Strange, pvc belongs to us, but we have no member for it. // Remove all finalizers, so it can be removed. log.Warn().Msg("PVC belongs to this deployment, but we don't know the member. Removing all finalizers") kubecli := r.context.GetKubeCli() ignoreNotFound := false - if err := k8sutil.RemovePVCFinalizers(log, kubecli, &p, p.GetFinalizers(), ignoreNotFound); err != nil { + if err := k8sutil.RemovePVCFinalizers(log, kubecli, pvc, pvc.GetFinalizers(), ignoreNotFound); err != nil { log.Debug().Err(err).Msg("Failed to update PVC (to remove all finalizers)") - return 0, maskAny(err) + return maskAny(err) } } - continue + return nil } - if k8sutil.IsPersistentVolumeClaimMarkedForDeletion(&p) { + if k8sutil.IsPersistentVolumeClaimMarkedForDeletion(pvc) { // Process finalizers - if x, err := r.runPVCFinalizers(ctx, &p, group, memberStatus); err != nil { + if x, err := r.runPVCFinalizers(ctx, pvc, group, memberStatus); err != nil { // Only log here, since we'll be called to try again. log.Warn().Err(err).Msg("Failed to run PVC finalizers") } else { nextInterval = nextInterval.ReduceTo(x) } } + + return nil + }, inspector.FilterPersistentVolumeClaimsByLabels(k8sutil.LabelsForDeployment(deploymentName, ""))); err != nil { + return 0, err } return nextInterval, nil diff --git a/pkg/deployment/resources/pvcs.go b/pkg/deployment/resources/pvcs.go index ad222cdb2..0410eb122 100644 --- a/pkg/deployment/resources/pvcs.go +++ b/pkg/deployment/resources/pvcs.go @@ -24,8 +24,7 @@ package resources import ( api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" "github.com/arangodb/kube-arangodb/pkg/util/constants" "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" ) @@ -36,7 +35,7 @@ func (r *Resources) createPVCFinalizers(group api.ServerGroup) []string { } // EnsurePVCs creates all PVC's listed in member status -func (r *Resources) EnsurePVCs() error { +func (r *Resources) EnsurePVCs(cachedStatus inspector.Inspector) error { kubecli := r.context.GetKubeCli() apiObject := r.context.GetAPIObject() deploymentName := apiObject.GetName() @@ -45,25 +44,24 @@ func (r *Resources) EnsurePVCs() error { iterator := r.context.GetServerGroupIterator() status, _ := r.context.GetStatus() enforceAntiAffinity := r.context.GetSpec().GetEnvironment().IsProduction() + pvcs := kubecli.CoreV1().PersistentVolumeClaims(apiObject.GetNamespace()) - pvcs := k8sutil.NewPersistentVolumeClaimCache(kubecli.CoreV1().PersistentVolumeClaims(ns)) if err := iterator.ForeachServerGroup(func(group api.ServerGroup, spec api.ServerGroupSpec, status *api.MemberStatusList) error { for _, m := range *status { if m.PersistentVolumeClaimName == "" { continue } - _, err := pvcs.Get(m.PersistentVolumeClaimName, metav1.GetOptions{}) - if k8sutil.IsNotFound(err) { - storageClassName := spec.GetStorageClassName() - role := group.AsRole() - resources := spec.Resources - vct := spec.VolumeClaimTemplate - finalizers := r.createPVCFinalizers(group) - if err := k8sutil.CreatePersistentVolumeClaim(pvcs, m.PersistentVolumeClaimName, deploymentName, ns, storageClassName, role, enforceAntiAffinity, resources, vct, finalizers, owner); err != nil { - return maskAny(err) - } - } else if err != nil { + _, exists := cachedStatus.PersistentVolumeClaim(m.PersistentVolumeClaimName) + if exists { + continue + } + storageClassName := spec.GetStorageClassName() + role := group.AsRole() + resources := spec.Resources + vct := spec.VolumeClaimTemplate + finalizers := r.createPVCFinalizers(group) + if err := k8sutil.CreatePersistentVolumeClaim(pvcs, m.PersistentVolumeClaimName, deploymentName, ns, storageClassName, role, enforceAntiAffinity, resources, vct, finalizers, owner); err != nil { return maskAny(err) } } diff --git a/pkg/deployment/resources/secret_hashes.go b/pkg/deployment/resources/secret_hashes.go index 63e43cabd..3d92ccbbd 100644 --- a/pkg/deployment/resources/secret_hashes.go +++ b/pkg/deployment/resources/secret_hashes.go @@ -30,6 +30,10 @@ import ( "sort" "strings" + "github.com/arangodb/kube-arangodb/pkg/deployment/pod" + + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + "github.com/arangodb/go-driver" v1 "k8s.io/api/core/v1" @@ -45,7 +49,7 @@ import ( // If a hash is different, the deployment is marked // with a SecretChangedCondition and the operator will not // touch it until this is resolved. -func (r *Resources) ValidateSecretHashes() error { +func (r *Resources) ValidateSecretHashes(cachedStatus inspector.Inspector) error { // validate performs a secret hash comparison for a single secret. // Return true if all is good, false when the SecretChanged condition // must be set. @@ -56,17 +60,13 @@ func (r *Resources) ValidateSecretHashes() error { log := r.log.With().Str("secret-name", secretName).Logger() expectedHash := getExpectedHash() - secret, hash, err := r.getSecretHash(secretName) + secret, hash, exists := r.getSecretHash(cachedStatus, secretName) if expectedHash == "" { // No hash set yet, try to fill it - if k8sutil.IsNotFound(err) { + if !exists { // Secret does not (yet) exists, do nothing return true, nil } - if err != nil { - log.Warn().Err(err).Msg("Failed to get secret") - return true, nil // Since we do not yet have a hash, we let this go with only a warning. - } // Hash fetched succesfully, store it if err := setExpectedHash(hash); err != nil { log.Debug().Msg("Failed to save secret hash") @@ -75,9 +75,9 @@ func (r *Resources) ValidateSecretHashes() error { return true, nil } // Hash is set, it must match the current hash - if err != nil { + if !exists { // Fetching error failed for other reason. - log.Debug().Err(err).Msg("Failed to fetch secret hash") + log.Debug().Msg("Secret does not exist") // This is not good, return false so SecretsChanged condition will be set. return false, nil } @@ -107,6 +107,7 @@ func (r *Resources) ValidateSecretHashes() error { } spec := r.context.GetSpec() + deploymentName := r.context.GetAPIObject().GetName() log := r.log var badSecretNames []string status, lastVersion := r.context.GetStatus() @@ -160,6 +161,19 @@ func (r *Resources) ValidateSecretHashes() error { } else if !hashOK { badSecretNames = append(badSecretNames, secretName) } + } else { + if _, exists := cachedStatus.Secret(pod.GetKeyfolderSecretName(deploymentName)); !exists { + secretName := spec.RocksDB.Encryption.GetKeySecretName() + getExpectedHash := func() string { return getHashes().RocksDBEncryptionKey } + setExpectedHash := func(h string) error { + return maskAny(updateHashes(func(dst *api.SecretHashes) { dst.RocksDBEncryptionKey = h })) + } + if hashOK, err := validate(secretName, getExpectedHash, setExpectedHash, nil); err != nil { + return maskAny(err) + } else if !hashOK { + badSecretNames = append(badSecretNames, secretName) + } + } } } if spec.IsSecure() { @@ -277,12 +291,10 @@ func changeUserPassword(c Context, secret *v1.Secret) error { } // getSecretHash fetches a secret with given name and returns a hash over its value. -func (r *Resources) getSecretHash(secretName string) (*v1.Secret, string, error) { - kubecli := r.context.GetKubeCli() - ns := r.context.GetNamespace() - s, err := kubecli.CoreV1().Secrets(ns).Get(secretName, metav1.GetOptions{}) - if err != nil { - return nil, "", maskAny(err) +func (r *Resources) getSecretHash(cachedStatus inspector.Inspector, secretName string) (*v1.Secret, string, bool) { + s, exists := cachedStatus.Secret(secretName) + if !exists { + return nil, "", false } // Create hash of value rows := make([]string, 0, len(s.Data)) @@ -294,5 +306,5 @@ func (r *Resources) getSecretHash(secretName string) (*v1.Secret, string, error) data := strings.Join(rows, "\n") rawHash := sha256.Sum256([]byte(data)) hash := fmt.Sprintf("%0x", rawHash) - return s, hash, nil + return s, hash, true } diff --git a/pkg/deployment/resources/secrets.go b/pkg/deployment/resources/secrets.go index 1957f5d7b..130114acd 100644 --- a/pkg/deployment/resources/secrets.go +++ b/pkg/deployment/resources/secrets.go @@ -24,9 +24,17 @@ package resources import ( "crypto/rand" + "crypto/sha256" "encoding/hex" + "fmt" "time" + operatorErrors "github.com/arangodb/kube-arangodb/pkg/util/errors" + + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + core "k8s.io/api/core/v1" + meta "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/arangodb/kube-arangodb/pkg/deployment/pod" "github.com/pkg/errors" @@ -35,8 +43,6 @@ import ( "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" - meta "k8s.io/apimachinery/pkg/apis/meta/v1" - api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" "github.com/arangodb/kube-arangodb/pkg/metrics" "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" @@ -48,12 +54,12 @@ var ( ) // EnsureSecrets creates all secrets needed to run the given deployment -func (r *Resources) EnsureSecrets() error { +func (r *Resources) EnsureSecrets(cachedStatus inspector.Inspector) error { start := time.Now() + spec := r.context.GetSpec() kubecli := r.context.GetKubeCli() ns := r.context.GetNamespace() secrets := k8sutil.NewSecretCache(kubecli.CoreV1().Secrets(ns)) - spec := r.context.GetSpec() status, _ := r.context.GetStatus() deploymentName := r.context.GetAPIObject().GetName() defer metrics.SetDuration(inspectSecretsDurationGauges.WithLabelValues(deploymentName), start) @@ -61,44 +67,44 @@ func (r *Resources) EnsureSecrets() error { if spec.IsAuthenticated() { counterMetric.Inc() - if err := r.ensureTokenSecret(secrets, spec.Authentication.GetJWTSecretName()); err != nil { + if err := r.ensureTokenSecret(cachedStatus, secrets, spec.Authentication.GetJWTSecretName()); err != nil { return maskAny(err) } if spec.Metrics.IsEnabled() { - if err := r.ensureExporterTokenSecret(secrets, spec.Metrics.GetJWTTokenSecretName(), spec.Authentication.GetJWTSecretName()); err != nil { + if err := r.ensureExporterTokenSecret(cachedStatus, secrets, spec.Metrics.GetJWTTokenSecretName(), spec.Authentication.GetJWTSecretName()); err != nil { return maskAny(err) } } } if spec.IsSecure() { counterMetric.Inc() - if err := r.ensureTLSCACertificateSecret(secrets, spec.TLS); err != nil { + if err := r.ensureTLSCACertificateSecret(cachedStatus, secrets, spec.TLS); err != nil { return maskAny(err) } } if spec.RocksDB.IsEncrypted() { if i := status.CurrentImage; i != nil && i.Enterprise && i.ArangoDBVersion.CompareTo("3.7.0") >= 0 { - if err := r.ensureEncryptionKeyfolderSecret(secrets, spec.RocksDB.Encryption.GetKeySecretName(), pod.GetKeyfolderSecretName(deploymentName)); err != nil { + if err := r.ensureEncryptionKeyfolderSecret(cachedStatus, secrets, spec.RocksDB.Encryption.GetKeySecretName(), pod.GetKeyfolderSecretName(deploymentName)); err != nil { return maskAny(err) } } } if spec.Sync.IsEnabled() { counterMetric.Inc() - if err := r.ensureTokenSecret(secrets, spec.Sync.Authentication.GetJWTSecretName()); err != nil { + if err := r.ensureTokenSecret(cachedStatus, secrets, spec.Sync.Authentication.GetJWTSecretName()); err != nil { return maskAny(err) } counterMetric.Inc() - if err := r.ensureTokenSecret(secrets, spec.Sync.Monitoring.GetTokenSecretName()); err != nil { + if err := r.ensureTokenSecret(cachedStatus, secrets, spec.Sync.Monitoring.GetTokenSecretName()); err != nil { return maskAny(err) } counterMetric.Inc() - if err := r.ensureTLSCACertificateSecret(secrets, spec.Sync.TLS); err != nil { + if err := r.ensureTLSCACertificateSecret(cachedStatus, secrets, spec.Sync.TLS); err != nil { return maskAny(err) } counterMetric.Inc() - if err := r.ensureClientAuthCACertificateSecret(secrets, spec.Sync.Authentication); err != nil { + if err := r.ensureClientAuthCACertificateSecret(cachedStatus, secrets, spec.Sync.Authentication); err != nil { return maskAny(err) } } @@ -108,34 +114,36 @@ func (r *Resources) EnsureSecrets() error { // ensureTokenSecret checks if a secret with given name exists in the namespace // of the deployment. If not, it will add such a secret with a random // token. -func (r *Resources) ensureTokenSecret(secrets k8sutil.SecretInterface, secretName string) error { - if _, err := secrets.Get(secretName, meta.GetOptions{}); k8sutil.IsNotFound(err) { - // Secret not found, create it - // Create token - tokenData := make([]byte, 32) - rand.Read(tokenData) - token := hex.EncodeToString(tokenData) +func (r *Resources) ensureTokenSecret(cachedStatus inspector.Inspector, secrets k8sutil.SecretInterface, secretName string) error { + if _, exists := cachedStatus.Secret(secretName); !exists { + return r.createTokenSecret(secrets, secretName) + } - // Create secret - owner := r.context.GetAPIObject().AsOwner() - if err := k8sutil.CreateTokenSecret(secrets, secretName, token, &owner); k8sutil.IsAlreadyExists(err) { - // Secret added while we tried it also - return nil - } else if err != nil { - // Failed to create secret - return maskAny(err) - } + return nil +} + +func (r *Resources) createTokenSecret(secrets k8sutil.SecretInterface, secretName string) error { + tokenData := make([]byte, 32) + rand.Read(tokenData) + token := hex.EncodeToString(tokenData) + + // Create secret + owner := r.context.GetAPIObject().AsOwner() + if err := k8sutil.CreateTokenSecret(secrets, secretName, token, &owner); k8sutil.IsAlreadyExists(err) { + // Secret added while we tried it also + return nil } else if err != nil { - // Failed to get secret for other reasons + // Failed to create secret return maskAny(err) } - return nil + + return operatorErrors.Reconcile() } -func (r *Resources) ensureEncryptionKeyfolderSecret(secrets k8sutil.SecretInterface, keyfileSecretName, secretName string) error { - keyfile, err := secrets.Get(keyfileSecretName, meta.GetOptions{}) - if err != nil { - return errors.Wrapf(err, "Unable to find original secret") +func (r *Resources) ensureEncryptionKeyfolderSecret(cachedStatus inspector.Inspector, secrets k8sutil.SecretInterface, keyfileSecretName, secretName string) error { + keyfile, exists := cachedStatus.Secret(keyfileSecretName) + if !exists { + return errors.Errorf("Unable to find original secret %s", keyfileSecretName) } if len(keyfile.Data) == 0 { @@ -148,12 +156,38 @@ func (r *Resources) ensureEncryptionKeyfolderSecret(secrets k8sutil.SecretInterf } owner := r.context.GetAPIObject().AsOwner() - if err = k8sutil.AppendKeyfileToKeyfolder(secrets, &owner, secretName, d); err != nil { + if err := AppendKeyfileToKeyfolder(cachedStatus, secrets, &owner, secretName, d); err != nil { return errors.Wrapf(err, "Unable to create keyfolder secret") } return nil } +func AppendKeyfileToKeyfolder(cachedStatus inspector.Inspector, secrets k8sutil.SecretInterface, ownerRef *meta.OwnerReference, secretName string, encryptionKey []byte) error { + encSha := fmt.Sprintf("%0x", sha256.Sum256(encryptionKey)) + if _, exists := cachedStatus.Secret(secretName); !exists { + + // Create secret + secret := &core.Secret{ + ObjectMeta: meta.ObjectMeta{ + Name: secretName, + }, + Data: map[string][]byte{ + encSha: encryptionKey, + }, + } + // Attach secret to owner + k8sutil.AddOwnerRefToObject(secret, ownerRef) + if _, err := secrets.Create(secret); err != nil { + // Failed to create secret + return maskAny(err) + } + + return operatorErrors.Reconcile() + } + + return nil +} + var ( exporterTokenClaims = map[string]interface{}{ "iss": "arangodb", @@ -164,8 +198,8 @@ var ( // ensureExporterTokenSecret checks if a secret with given name exists in the namespace // of the deployment. If not, it will add such a secret with correct access. -func (r *Resources) ensureExporterTokenSecret(secrets k8sutil.SecretInterface, tokenSecretName, secretSecretName string) error { - if recreate, exists, err := r.ensureExporterTokenSecretCreateRequired(secrets, tokenSecretName, secretSecretName); err != nil { +func (r *Resources) ensureExporterTokenSecret(cachedStatus inspector.Inspector, secrets k8sutil.SecretInterface, tokenSecretName, secretSecretName string) error { + if recreate, exists, err := r.ensureExporterTokenSecretCreateRequired(cachedStatus, tokenSecretName, secretSecretName); err != nil { return err } else if recreate { // Create secret @@ -183,21 +217,28 @@ func (r *Resources) ensureExporterTokenSecret(secrets k8sutil.SecretInterface, t // Failed to create secret return maskAny(err) } + + return operatorErrors.Reconcile() } return nil } -func (r *Resources) ensureExporterTokenSecretCreateRequired(secrets k8sutil.SecretInterface, tokenSecretName, secretSecretName string) (bool, bool, error) { - if secret, err := secrets.Get(tokenSecretName, meta.GetOptions{}); k8sutil.IsNotFound(err) { +func (r *Resources) ensureExporterTokenSecretCreateRequired(cachedStatus inspector.Inspector, tokenSecretName, secretSecretName string) (bool, bool, error) { + if secret, exists := cachedStatus.Secret(tokenSecretName); !exists { return true, false, nil - } else if err == nil { + } else { // Check if claims are fine data, ok := secret.Data[constants.SecretKeyToken] if !ok { return true, true, nil } - secret, err := k8sutil.GetTokenSecret(secrets, secretSecretName) + jwtSecret, exists := cachedStatus.Secret(secretSecretName) + if !exists { + return false, true, errors.Errorf("Secret %s does not exists", secretSecretName) + } + + secret, err := k8sutil.GetTokenFromSecret(jwtSecret) if err != nil { return false, true, maskAny(err) } @@ -216,16 +257,13 @@ func (r *Resources) ensureExporterTokenSecretCreateRequired(secrets k8sutil.Secr } return !equality.Semantic.DeepEqual(tokenClaims, exporterTokenClaims), true, nil - } else { - // Failed to get secret for other reasons - return false, false, maskAny(err) } } // ensureTLSCACertificateSecret checks if a secret with given name exists in the namespace // of the deployment. If not, it will add such a secret with a generated CA certificate. -func (r *Resources) ensureTLSCACertificateSecret(secrets k8sutil.SecretInterface, spec api.TLSSpec) error { - if _, err := secrets.Get(spec.GetCASecretName(), meta.GetOptions{}); k8sutil.IsNotFound(err) { +func (r *Resources) ensureTLSCACertificateSecret(cachedStatus inspector.Inspector, secrets k8sutil.SecretInterface, spec api.TLSSpec) error { + if _, exists := cachedStatus.Secret(spec.GetCASecretName()); !exists { // Secret not found, create it apiObject := r.context.GetAPIObject() owner := apiObject.AsOwner() @@ -237,17 +275,16 @@ func (r *Resources) ensureTLSCACertificateSecret(secrets k8sutil.SecretInterface // Failed to create secret return maskAny(err) } - } else if err != nil { - // Failed to get secret for other reasons - return maskAny(err) + + return operatorErrors.Reconcile() } return nil } // ensureClientAuthCACertificateSecret checks if a secret with given name exists in the namespace // of the deployment. If not, it will add such a secret with a generated CA certificate. -func (r *Resources) ensureClientAuthCACertificateSecret(secrets k8sutil.SecretInterface, spec api.SyncAuthenticationSpec) error { - if _, err := secrets.Get(spec.GetClientCASecretName(), meta.GetOptions{}); k8sutil.IsNotFound(err) { +func (r *Resources) ensureClientAuthCACertificateSecret(cachedStatus inspector.Inspector, secrets k8sutil.SecretInterface, spec api.SyncAuthenticationSpec) error { + if _, exists := cachedStatus.Secret(spec.GetClientCASecretName()); !exists { // Secret not found, create it apiObject := r.context.GetAPIObject() owner := apiObject.AsOwner() @@ -259,9 +296,8 @@ func (r *Resources) ensureClientAuthCACertificateSecret(secrets k8sutil.SecretIn // Failed to create secret return maskAny(err) } - } else if err != nil { - // Failed to get secret for other reasons - return maskAny(err) + + return operatorErrors.Reconcile() } return nil } diff --git a/pkg/deployment/resources/services.go b/pkg/deployment/resources/services.go index 74c6e77b1..735fe3f35 100644 --- a/pkg/deployment/resources/services.go +++ b/pkg/deployment/resources/services.go @@ -26,6 +26,8 @@ import ( "strings" "time" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -42,7 +44,7 @@ var ( ) // EnsureServices creates all services needed to service the deployment -func (r *Resources) EnsureServices() error { +func (r *Resources) EnsureServices(cachedStatus inspector.Inspector) error { log := r.log start := time.Now() kubecli := r.context.GetKubeCli() @@ -55,10 +57,10 @@ func (r *Resources) EnsureServices() error { counterMetric := inspectedServicesCounters.WithLabelValues(deploymentName) // Fetch existing services - svcs := k8sutil.NewServiceCache(kubecli.CoreV1().Services(ns)) + svcs := kubecli.CoreV1().Services(ns) // Headless service counterMetric.Inc() - if _, err := svcs.Get(k8sutil.CreateHeadlessServiceName(deploymentName), metav1.GetOptions{}); err != nil { + if _, exists := cachedStatus.Service(k8sutil.CreateHeadlessServiceName(deploymentName)); !exists { svcName, newlyCreated, err := k8sutil.CreateHeadlessService(svcs, apiObject, owner) if err != nil { log.Debug().Err(err).Msg("Failed to create headless service") @@ -72,7 +74,7 @@ func (r *Resources) EnsureServices() error { // Internal database client service single := spec.GetMode().HasSingleServers() counterMetric.Inc() - if _, err := svcs.Get(k8sutil.CreateDatabaseClientServiceName(deploymentName), metav1.GetOptions{}); err != nil { + if _, exists := cachedStatus.Service(k8sutil.CreateDatabaseClientServiceName(deploymentName)); !exists { svcName, newlyCreated, err := k8sutil.CreateDatabaseClientService(svcs, apiObject, single, owner) if err != nil { log.Debug().Err(err).Msg("Failed to create database client service") @@ -98,7 +100,7 @@ func (r *Resources) EnsureServices() error { if single { role = "single" } - if err := r.ensureExternalAccessServices(svcs, eaServiceName, ns, role, "database", k8sutil.ArangoPort, false, spec.ExternalAccess, apiObject, log, counterMetric); err != nil { + if err := r.ensureExternalAccessServices(cachedStatus, svcs, eaServiceName, ns, role, "database", k8sutil.ArangoPort, false, spec.ExternalAccess, apiObject, log, counterMetric); err != nil { return maskAny(err) } @@ -107,7 +109,7 @@ func (r *Resources) EnsureServices() error { counterMetric.Inc() eaServiceName := k8sutil.CreateSyncMasterClientServiceName(deploymentName) role := "syncmaster" - if err := r.ensureExternalAccessServices(svcs, eaServiceName, ns, role, "sync", k8sutil.ArangoSyncMasterPort, true, spec.Sync.ExternalAccess.ExternalAccessSpec, apiObject, log, counterMetric); err != nil { + if err := r.ensureExternalAccessServices(cachedStatus, svcs, eaServiceName, ns, role, "sync", k8sutil.ArangoSyncMasterPort, true, spec.Sync.ExternalAccess.ExternalAccessSpec, apiObject, log, counterMetric); err != nil { return maskAny(err) } status, lastVersion := r.context.GetStatus() @@ -120,7 +122,7 @@ func (r *Resources) EnsureServices() error { } if spec.Metrics.IsEnabled() { - name, _, err := k8sutil.CreateExporterService(svcs, apiObject, apiObject.AsOwner()) + name, _, err := k8sutil.CreateExporterService(cachedStatus, svcs, apiObject, apiObject.AsOwner()) if err != nil { log.Debug().Err(err).Msgf("Failed to create %s exporter service", name) return maskAny(err) @@ -137,12 +139,12 @@ func (r *Resources) EnsureServices() error { } // EnsureServices creates all services needed to service the deployment -func (r *Resources) ensureExternalAccessServices(svcs k8sutil.ServiceInterface, eaServiceName, ns, svcRole, title string, port int, noneIsClusterIP bool, spec api.ExternalAccessSpec, apiObject k8sutil.APIObject, log zerolog.Logger, counterMetric prometheus.Counter) error { +func (r *Resources) ensureExternalAccessServices(cachedStatus inspector.Inspector, svcs k8sutil.ServiceInterface, eaServiceName, ns, svcRole, title string, port int, noneIsClusterIP bool, spec api.ExternalAccessSpec, apiObject k8sutil.APIObject, log zerolog.Logger, counterMetric prometheus.Counter) error { // Database external access service createExternalAccessService := false deleteExternalAccessService := false eaServiceType := spec.GetType().AsServiceType() // Note: Type auto defaults to ServiceTypeLoadBalancer - if existing, err := svcs.Get(eaServiceName, metav1.GetOptions{}); err == nil { + if existing, exists := cachedStatus.Service(eaServiceName); exists { // External access service exists updateExternalAccessService := false loadBalancerIP := spec.GetLoadBalancerIP() @@ -198,12 +200,13 @@ func (r *Resources) ensureExternalAccessServices(svcs k8sutil.ServiceInterface, return maskAny(err) } } - } else if k8sutil.IsNotFound(err) { + } else { // External access service does not exist if !spec.GetType().IsNone() || noneIsClusterIP { createExternalAccessService = true } } + if deleteExternalAccessService { log.Info().Str("service", eaServiceName).Msgf("Removing obsolete %s external access service", title) if err := svcs.Delete(eaServiceName, &metav1.DeleteOptions{}); err != nil { diff --git a/pkg/operator/errors.go b/pkg/operator/errors.go index 4ce79f6f3..a0b41b610 100644 --- a/pkg/operator/errors.go +++ b/pkg/operator/errors.go @@ -22,8 +22,15 @@ package operator -import "github.com/pkg/errors" - -var ( - maskAny = errors.WithStack +import ( + operatorErrors "github.com/arangodb/kube-arangodb/pkg/util/errors" + "github.com/pkg/errors" ) + +func maskAny(err error) error { + if operatorErrors.IsReconcile(err) { + return nil + } + + return errors.WithStack(err) +} diff --git a/pkg/util/errors/errors.go b/pkg/util/errors/errors.go index d2267c769..687ce31e1 100644 --- a/pkg/util/errors/errors.go +++ b/pkg/util/errors/errors.go @@ -209,3 +209,34 @@ func LogError(logger zerolog.Logger, msg string, f func() error) { logger.Error().Err(err).Msg(msg) } } + +type causer interface { + Cause() error +} + +func IsReconcile(err error) bool { + if err == nil { + return false + } + + if _, ok := err.(reconcile); ok { + return true + } + + if c, ok := err.(causer); ok { + return IsReconcile(c.Cause()) + } + + return false +} + +func Reconcile() error { + return reconcile{} +} + +type reconcile struct { +} + +func (r reconcile) Error() string { + return "reconcile" +} diff --git a/pkg/util/k8sutil/interfaces/pod_creator.go b/pkg/util/k8sutil/interfaces/pod_creator.go new file mode 100644 index 000000000..581b084f2 --- /dev/null +++ b/pkg/util/k8sutil/interfaces/pod_creator.go @@ -0,0 +1,60 @@ +// +// DISCLAIMER +// +// Copyright 2020 ArangoDB GmbH, Cologne, Germany +// +// 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. +// +// Copyright holder is ArangoDB GmbH, Cologne, Germany +// +// Author Adam Janikowski +// + +package interfaces + +import ( + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + core "k8s.io/api/core/v1" +) + +type PodCreator interface { + Init(*core.Pod) + GetName() string + GetRole() string + GetVolumes() ([]core.Volume, []core.VolumeMount) + GetSidecars(*core.Pod) + GetInitContainers() ([]core.Container, error) + GetFinalizers() []string + GetTolerations() []core.Toleration + GetNodeSelector() map[string]string + GetServiceAccountName() string + GetPodAntiAffinity() *core.PodAntiAffinity + GetPodAffinity() *core.PodAffinity + GetNodeAffinity() *core.NodeAffinity + GetContainerCreator() ContainerCreator + GetImagePullSecrets() []string + IsDeploymentMode() bool + Validate(cachedStatus inspector.Inspector) error +} + +type ContainerCreator interface { + GetExecutor() string + GetProbes() (*core.Probe, *core.Probe, error) + GetResourceRequirements() core.ResourceRequirements + GetLifecycle() (*core.Lifecycle, error) + GetImagePullPolicy() core.PullPolicy + GetImage() string + GetEnvs() []core.EnvVar + GetSecurityContext() *core.SecurityContext + GetPorts() []core.ContainerPort +} diff --git a/pkg/util/k8sutil/pods.go b/pkg/util/k8sutil/pods.go index 1688e4e89..2dbf27a12 100644 --- a/pkg/util/k8sutil/pods.go +++ b/pkg/util/k8sutil/pods.go @@ -28,6 +28,8 @@ import ( "path/filepath" "time" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/interfaces" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/json" @@ -58,38 +60,6 @@ const ( MasterJWTSecretVolumeMountDir = "/secrets/master/jwt" ) -type PodCreator interface { - Init(*core.Pod) - GetName() string - GetRole() string - GetVolumes() ([]core.Volume, []core.VolumeMount) - GetSidecars(*core.Pod) - GetInitContainers() ([]core.Container, error) - GetFinalizers() []string - GetTolerations() []core.Toleration - GetNodeSelector() map[string]string - GetServiceAccountName() string - GetPodAntiAffinity() *core.PodAntiAffinity - GetPodAffinity() *core.PodAffinity - GetNodeAffinity() *core.NodeAffinity - GetContainerCreator() ContainerCreator - GetImagePullSecrets() []string - IsDeploymentMode() bool - Validate(secrets SecretInterface) error -} - -type ContainerCreator interface { - GetExecutor() string - GetProbes() (*core.Probe, *core.Probe, error) - GetResourceRequirements() core.ResourceRequirements - GetLifecycle() (*core.Lifecycle, error) - GetImagePullPolicy() core.PullPolicy - GetImage() string - GetEnvs() []core.EnvVar - GetSecurityContext() *core.SecurityContext - GetPorts() []core.ContainerPort -} - // IsPodReady returns true if the PodReady condition on // the given pod is set to true. func IsPodReady(pod *core.Pod) bool { @@ -176,7 +146,7 @@ func IsPodTerminating(pod *core.Pod) bool { } // IsArangoDBImageIDAndVersionPod returns true if the given pod is used for fetching image ID and ArangoDB version of an image -func IsArangoDBImageIDAndVersionPod(p core.Pod) bool { +func IsArangoDBImageIDAndVersionPod(p *core.Pod) bool { role, found := p.GetLabels()[LabelKeyRole] return found && role == ImageIDAndVersionRole } @@ -339,7 +309,7 @@ func ExtractPodResourceRequirement(resources core.ResourceRequirements) core.Res } // NewContainer creates a container for specified creator -func NewContainer(args []string, containerCreator ContainerCreator) (core.Container, error) { +func NewContainer(args []string, containerCreator interfaces.ContainerCreator) (core.Container, error) { liveness, readiness, err := containerCreator.GetProbes() if err != nil { @@ -367,7 +337,7 @@ func NewContainer(args []string, containerCreator ContainerCreator) (core.Contai } // NewPod creates a basic Pod for given settings. -func NewPod(deploymentName, role, id, podName string, podCreator PodCreator) core.Pod { +func NewPod(deploymentName, role, id, podName string, podCreator interfaces.PodCreator) core.Pod { hostname := CreatePodHostName(deploymentName, role, id) p := core.Pod{ @@ -418,7 +388,7 @@ func GetPodSpecChecksum(podSpec core.PodSpec) (string, error) { // If the pod already exists, nil is returned. // If another error occurs, that error is returned. func CreatePod(kubecli kubernetes.Interface, pod *core.Pod, ns string, owner metav1.OwnerReference) (types.UID, string, error) { - addOwnerRefToObject(pod.GetObjectMeta(), &owner) + AddOwnerRefToObject(pod.GetObjectMeta(), &owner) checksum, err := GetPodSpecChecksum(pod.Spec) if err != nil { diff --git a/pkg/util/k8sutil/pvc.go b/pkg/util/k8sutil/pvc.go index ef3fa9709..c9e6c2c10 100644 --- a/pkg/util/k8sutil/pvc.go +++ b/pkg/util/k8sutil/pvc.go @@ -109,7 +109,7 @@ func CreatePersistentVolumeClaim(pvcs PersistentVolumeClaimInterface, pvcName, d if storageClassName != "" { pvc.Spec.StorageClassName = &storageClassName } - addOwnerRefToObject(pvc.GetObjectMeta(), &owner) + AddOwnerRefToObject(pvc.GetObjectMeta(), &owner) if _, err := pvcs.Create(pvc); err != nil && !IsAlreadyExists(err) { return maskAny(err) } diff --git a/pkg/util/k8sutil/secrets.go b/pkg/util/k8sutil/secrets.go index 5e942c181..9461163cd 100644 --- a/pkg/util/k8sutil/secrets.go +++ b/pkg/util/k8sutil/secrets.go @@ -23,9 +23,6 @@ package k8sutil import ( - "crypto/sha256" - "fmt" - goErrors "github.com/pkg/errors" core "k8s.io/api/core/v1" meta "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -51,13 +48,17 @@ func ValidateEncryptionKeySecret(secrets SecretInterface, secretName string) err if err != nil { return maskAny(err) } + return ValidateEncryptionKeyFromSecret(s) +} + +func ValidateEncryptionKeyFromSecret(s *core.Secret) error { // Check `key` field keyData, found := s.Data[constants.SecretEncryptionKey] if !found { - return maskAny(fmt.Errorf("No '%s' found in secret '%s'", constants.SecretEncryptionKey, secretName)) + return maskAny(goErrors.Errorf("No '%s' found in secret '%s'", constants.SecretEncryptionKey, s.GetName())) } if len(keyData) != 32 { - return maskAny(fmt.Errorf("'%s' in secret '%s' is expected to be 32 bytes long, found %d", constants.SecretEncryptionKey, secretName, len(keyData))) + return maskAny(goErrors.Errorf("'%s' in secret '%s' is expected to be 32 bytes long, found %d", constants.SecretEncryptionKey, s.GetName(), len(keyData))) } return nil } @@ -65,7 +66,7 @@ func ValidateEncryptionKeySecret(secrets SecretInterface, secretName string) err // CreateEncryptionKeySecret creates a secret used to store a RocksDB encryption key. func CreateEncryptionKeySecret(secrets SecretInterface, secretName string, key []byte) error { if len(key) != 32 { - return maskAny(fmt.Errorf("Key in secret '%s' is expected to be 32 bytes long, got %d", secretName, len(key))) + return maskAny(goErrors.Errorf("Key in secret '%s' is expected to be 32 bytes long, got %d", secretName, len(key))) } // Create secret secret := &core.Secret{ @@ -93,7 +94,7 @@ func ValidateCACertificateSecret(secrets SecretInterface, secretName string) err // Check `ca.crt` field _, found := s.Data[constants.SecretCACertificate] if !found { - return maskAny(fmt.Errorf("No '%s' found in secret '%s'", constants.SecretCACertificate, secretName)) + return maskAny(goErrors.Errorf("No '%s' found in secret '%s'", constants.SecretCACertificate, secretName)) } return nil } @@ -111,7 +112,7 @@ func GetCACertficateSecret(secrets SecretInterface, secretName string) (string, // Load `ca.crt` field cert, found := s.Data[constants.SecretCACertificate] if !found { - return "", maskAny(fmt.Errorf("No '%s' found in secret '%s'", constants.SecretCACertificate, secretName)) + return "", maskAny(goErrors.Errorf("No '%s' found in secret '%s'", constants.SecretCACertificate, secretName)) } return string(cert), nil } @@ -126,6 +127,10 @@ func GetCASecret(secrets SecretInterface, secretName string, ownerRef *meta.Owne if err != nil { return "", "", false, maskAny(err) } + return GetCAFromSecret(s, ownerRef) +} + +func GetCAFromSecret(s *core.Secret, ownerRef *meta.OwnerReference) (string, string, bool, error) { isOwned := false if ownerRef != nil { for _, x := range s.GetOwnerReferences() { @@ -138,11 +143,11 @@ func GetCASecret(secrets SecretInterface, secretName string, ownerRef *meta.Owne // Load `ca.crt` field cert, found := s.Data[constants.SecretCACertificate] if !found { - return "", "", isOwned, maskAny(fmt.Errorf("No '%s' found in secret '%s'", constants.SecretCACertificate, secretName)) + return "", "", isOwned, maskAny(goErrors.Errorf("No '%s' found in secret '%s'", constants.SecretCACertificate, s.GetName())) } priv, found := s.Data[constants.SecretCAKey] if !found { - return "", "", isOwned, maskAny(fmt.Errorf("No '%s' found in secret '%s'", constants.SecretCAKey, secretName)) + return "", "", isOwned, maskAny(goErrors.Errorf("No '%s' found in secret '%s'", constants.SecretCAKey, s.GetName())) } return string(cert), string(priv), isOwned, nil } @@ -160,7 +165,7 @@ func CreateCASecret(secrets SecretInterface, secretName string, certificate, key }, } // Attach secret to owner - addOwnerRefToObject(secret, ownerRef) + AddOwnerRefToObject(secret, ownerRef) if _, err := secrets.Create(secret); err != nil { // Failed to create secret return maskAny(err) @@ -176,10 +181,14 @@ func GetTLSKeyfileSecret(secrets SecretInterface, secretName string) (string, er if err != nil { return "", maskAny(err) } + return GetTLSKeyfileFromSecret(s) +} + +func GetTLSKeyfileFromSecret(s *core.Secret) (string, error) { // Load `tls.keyfile` field keyfile, found := s.Data[constants.SecretTLSKeyfile] if !found { - return "", maskAny(fmt.Errorf("No '%s' found in secret '%s'", constants.SecretTLSKeyfile, secretName)) + return "", maskAny(goErrors.Errorf("No '%s' found in secret '%s'", constants.SecretTLSKeyfile, s.GetName())) } return string(keyfile), nil } @@ -197,7 +206,7 @@ func CreateTLSKeyfileSecret(secrets SecretInterface, secretName string, keyfile }, } // Attach secret to owner - addOwnerRefToObject(secret, ownerRef) + AddOwnerRefToObject(secret, ownerRef) if _, err := secrets.Create(secret); err != nil { // Failed to create secret return maskAny(err) @@ -212,10 +221,14 @@ func ValidateTokenSecret(secrets SecretInterface, secretName string) error { if err != nil { return maskAny(err) } + return ValidateTokenFromSecret(s) +} + +func ValidateTokenFromSecret(s *core.Secret) error { // Check `token` field _, found := s.Data[constants.SecretKeyToken] if !found { - return maskAny(fmt.Errorf("No '%s' found in secret '%s'", constants.SecretKeyToken, secretName)) + return maskAny(goErrors.Errorf("No '%s' found in secret '%s'", constants.SecretKeyToken, s.GetName())) } return nil } @@ -226,10 +239,15 @@ func GetTokenSecret(secrets SecretInterface, secretName string) (string, error) if err != nil { return "", maskAny(err) } + return GetTokenFromSecret(s) +} + +// GetTokenFromSecret loads the token secret from a Secret with given name. +func GetTokenFromSecret(s *core.Secret) (string, error) { // Take the first data from the token key data, found := s.Data[constants.SecretKeyToken] if !found { - return "", maskAny(fmt.Errorf("No '%s' data found in secret '%s'", constants.SecretKeyToken, secretName)) + return "", maskAny(goErrors.Errorf("No '%s' data found in secret '%s'", constants.SecretKeyToken, s.GetName())) } return string(data), nil } @@ -247,7 +265,7 @@ func CreateTokenSecret(secrets SecretInterface, secretName, token string, ownerR }, } // Attach secret to owner - addOwnerRefToObject(secret, ownerRef) + AddOwnerRefToObject(secret, ownerRef) if _, err := secrets.Create(secret); err != nil { // Failed to create secret return maskAny(err) @@ -255,35 +273,6 @@ func CreateTokenSecret(secrets SecretInterface, secretName, token string, ownerR return nil } -func AppendKeyfileToKeyfolder(secrets SecretInterface, ownerRef *meta.OwnerReference, secretName string, encryptionKey []byte) error { - encSha := fmt.Sprintf("%0x", sha256.Sum256(encryptionKey)) - if _, err := secrets.Get(secretName, meta.GetOptions{}); err != nil { - if !IsNotFound(err) { - return goErrors.Wrapf(err, "Unable to get keyfolder") - } - - // Create secret - secret := &core.Secret{ - ObjectMeta: meta.ObjectMeta{ - Name: secretName, - }, - Data: map[string][]byte{ - encSha: encryptionKey, - }, - } - // Attach secret to owner - addOwnerRefToObject(secret, ownerRef) - if _, err := secrets.Create(secret); err != nil { - // Failed to create secret - return maskAny(err) - } - - return nil - } - - return nil -} - // CreateJWTFromSecret creates a JWT using the secret stored in secretSecretName and stores the // result in a new secret called tokenSecretName func CreateJWTFromSecret(secrets SecretInterface, tokenSecretName, secretSecretName string, claims map[string]interface{}, ownerRef *meta.OwnerReference) error { @@ -319,7 +308,7 @@ func CreateBasicAuthSecret(secrets SecretInterface, secretName, username, passwo }, } // Attach secret to owner - addOwnerRefToObject(secret, ownerRef) + AddOwnerRefToObject(secret, ownerRef) if _, err := secrets.Create(secret); err != nil { // Failed to create secret return maskAny(err) @@ -344,11 +333,11 @@ func GetBasicAuthSecret(secrets SecretInterface, secretName string) (string, str func GetSecretAuthCredentials(secret *core.Secret) (string, string, error) { username, found := secret.Data[constants.SecretUsername] if !found { - return "", "", maskAny(fmt.Errorf("No '%s' found in secret '%s'", constants.SecretUsername, secret.Name)) + return "", "", maskAny(goErrors.Errorf("No '%s' found in secret '%s'", constants.SecretUsername, secret.Name)) } password, found := secret.Data[constants.SecretPassword] if !found { - return "", "", maskAny(fmt.Errorf("No '%s' found in secret '%s'", constants.SecretPassword, secret.Name)) + return "", "", maskAny(goErrors.Errorf("No '%s' found in secret '%s'", constants.SecretPassword, secret.Name)) } return string(username), string(password), nil } diff --git a/pkg/util/k8sutil/services.go b/pkg/util/k8sutil/services.go index bd5a0ea9f..6bbfc1c23 100644 --- a/pkg/util/k8sutil/services.go +++ b/pkg/util/k8sutil/services.go @@ -29,6 +29,8 @@ import ( "strconv" "strings" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -72,12 +74,16 @@ func CreateExporterClientServiceName(deploymentName string) string { } // CreateExporterService -func CreateExporterService(svcs ServiceInterface, deployment metav1.Object, owner metav1.OwnerReference) (string, bool, error) { +func CreateExporterService(cachedStatus inspector.Inspector, svcs ServiceInterface, deployment metav1.Object, owner metav1.OwnerReference) (string, bool, error) { deploymentName := deployment.GetName() svcName := CreateExporterClientServiceName(deploymentName) selectorLabels := LabelsForExporterServiceSelector(deploymentName) + if _, exists := cachedStatus.Service(svcName); exists { + return svcName, false, nil + } + svc := &v1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: svcName, @@ -95,7 +101,7 @@ func CreateExporterService(svcs ServiceInterface, deployment metav1.Object, owne Selector: selectorLabels, }, } - addOwnerRefToObject(svc.GetObjectMeta(), &owner) + AddOwnerRefToObject(svc.GetObjectMeta(), &owner) if _, err := svcs.Create(svc); IsAlreadyExists(err) { return svcName, false, nil } else if err != nil { @@ -202,7 +208,7 @@ func createService(svcs ServiceInterface, svcName, deploymentName, ns, clusterIP LoadBalancerSourceRanges: loadBalancerSourceRanges, }, } - addOwnerRefToObject(svc.GetObjectMeta(), &owner) + AddOwnerRefToObject(svc.GetObjectMeta(), &owner) if _, err := svcs.Create(svc); IsAlreadyExists(err) { return false, nil } else if err != nil { diff --git a/pkg/util/k8sutil/util.go b/pkg/util/k8sutil/util.go index 69d277a44..b2ff111f3 100644 --- a/pkg/util/k8sutil/util.go +++ b/pkg/util/k8sutil/util.go @@ -43,8 +43,8 @@ const ( AppName = "arangodb" ) -// addOwnerRefToObject adds given owner reference to given object -func addOwnerRefToObject(obj metav1.Object, ownerRef *metav1.OwnerReference) { +// AddOwnerRefToObject adds given owner reference to given object +func AddOwnerRefToObject(obj metav1.Object, ownerRef *metav1.OwnerReference) { if ownerRef != nil { obj.SetOwnerReferences(append(obj.GetOwnerReferences(), *ownerRef)) } diff --git a/pkg/util/k8sutil/util_test.go b/pkg/util/k8sutil/util_test.go index 28242deb7..d15da0062 100644 --- a/pkg/util/k8sutil/util_test.go +++ b/pkg/util/k8sutil/util_test.go @@ -30,13 +30,13 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// TestAddOwnerRefToObject tests addOwnerRefToObject. +// TestAddOwnerRefToObject tests AddOwnerRefToObject. func TestAddOwnerRefToObject(t *testing.T) { p := &v1.Pod{} - addOwnerRefToObject(p, nil) + AddOwnerRefToObject(p, nil) assert.Len(t, p.GetOwnerReferences(), 0) - addOwnerRefToObject(p, &metav1.OwnerReference{}) + AddOwnerRefToObject(p, &metav1.OwnerReference{}) assert.Len(t, p.GetOwnerReferences(), 1) } From 859f01aee78a426d776cecbdf66c5b238f3a51dd Mon Sep 17 00:00:00 2001 From: ajanikow <12255597+ajanikow@users.noreply.github.com> Date: Fri, 5 Jun 2020 14:06:34 +0000 Subject: [PATCH 2/3] Fix tests --- pkg/deployment/deployment_inspector.go | 33 +++++++++---------- pkg/deployment/deployment_run_test.go | 33 ++++++++++++++++--- pkg/deployment/reconcile/plan_builder.go | 10 +++--- pkg/deployment/reconcile/plan_builder_test.go | 8 ++--- .../resources/inspector/inspector.go | 6 ++-- pkg/deployment/resources/secrets.go | 12 +++---- 6 files changed, 62 insertions(+), 40 deletions(-) diff --git a/pkg/deployment/deployment_inspector.go b/pkg/deployment/deployment_inspector.go index c97c9c4b6..1fa0aa89e 100644 --- a/pkg/deployment/deployment_inspector.go +++ b/pkg/deployment/deployment_inspector.go @@ -65,13 +65,12 @@ func (d *Deployment) inspectDeployment(lastInterval util.Interval) util.Interval deploymentName := d.apiObject.GetName() defer metrics.SetDuration(inspectDeploymentDurationGauges.WithLabelValues(deploymentName), start) - cachedStatus, err := inspector.NewInspector(d.GetKubeCli(), d.GetNamespace()) + cachedStatus, err := inspector.NewInspector(d.GetKubeCli(), d.GetNamespace()) if err != nil { log.Error().Err(err).Msg("Unable to get resources") return minInspectionInterval // Retry ASAP } - // Check deployment still exists updated, err := d.deps.DatabaseCRCli.DatabaseV1().ArangoDeployments(d.apiObject.GetNamespace()).Get(deploymentName, metav1.GetOptions{}) if k8sutil.IsNotFound(err) { @@ -151,21 +150,21 @@ func (d *Deployment) inspectDeploymentWithError(ctx context.Context, lastInterva } } - if err := d.resources.EnsureSecrets(cachedStatus); err != nil { + if err := d.resources.EnsureSecrets(cachedStatus); err != nil { return minInspectionInterval, errors.Wrapf(err, "Secret creation failed") } - if err := d.resources.EnsureServices(cachedStatus); err != nil { + if err := d.resources.EnsureServices(cachedStatus); err != nil { return minInspectionInterval, errors.Wrapf(err, "Service creation failed") } // Inspect secret hashes - if err := d.resources.ValidateSecretHashes(cachedStatus); err != nil { + if err := d.resources.ValidateSecretHashes(cachedStatus); err != nil { return minInspectionInterval, errors.Wrapf(err, "Secret hash validation failed") } // Check for LicenseKeySecret - if err := d.resources.ValidateLicenseKeySecret(cachedStatus); err != nil { + if err := d.resources.ValidateLicenseKeySecret(cachedStatus); err != nil { return minInspectionInterval, errors.Wrapf(err, "License Key Secret invalid") } @@ -175,43 +174,43 @@ func (d *Deployment) inspectDeploymentWithError(ctx context.Context, lastInterva } // Ensure we have image info - if retrySoon, err := d.ensureImages(d.apiObject); err != nil { + if retrySoon, err := d.ensureImages(d.apiObject); err != nil { return minInspectionInterval, errors.Wrapf(err, "Image detection failed") } else if retrySoon { return minInspectionInterval, nil } // Inspection of generated resources needed - if x, err := d.resources.InspectPods(ctx, cachedStatus); err != nil { + if x, err := d.resources.InspectPods(ctx, cachedStatus); err != nil { return minInspectionInterval, errors.Wrapf(err, "Pod inspection failed") } else { nextInterval = nextInterval.ReduceTo(x) } - if x, err := d.resources.InspectPVCs(ctx, cachedStatus); err != nil { + if x, err := d.resources.InspectPVCs(ctx, cachedStatus); err != nil { return minInspectionInterval, errors.Wrapf(err, "PVC inspection failed") } else { nextInterval = nextInterval.ReduceTo(x) } // Check members for resilience - if err := d.resilience.CheckMemberFailure(); err != nil { + if err := d.resilience.CheckMemberFailure(); err != nil { return minInspectionInterval, errors.Wrapf(err, "Member failure detection failed") } // Immediate actions - if err := d.reconciler.CheckDeployment(); err != nil { + if err := d.reconciler.CheckDeployment(); err != nil { return minInspectionInterval, errors.Wrapf(err, "Reconciler immediate actions failed") } - if interval, err := d.ensureResources(nextInterval, cachedStatus); err != nil { + if interval, err := d.ensureResources(nextInterval, cachedStatus); err != nil { return minInspectionInterval, errors.Wrapf(err, "Reconciler resource recreation failed") } else { nextInterval = interval } // Create scale/update plan - if err, updated := d.reconciler.CreatePlan(ctx, cachedStatus); err != nil { + if err, updated := d.reconciler.CreatePlan(ctx, cachedStatus); err != nil { return minInspectionInterval, errors.Wrapf(err, "Plan creation failed") } else if updated { return minInspectionInterval, nil @@ -286,19 +285,19 @@ func (d *Deployment) ensureResources(lastInterval util.Interval, cachedStatus in } } - if err := d.resources.EnsurePVCs(cachedStatus); err != nil { + if err := d.resources.EnsurePVCs(cachedStatus); err != nil { return minInspectionInterval, errors.Wrapf(err, "PVC creation failed") } - if err := d.resources.EnsurePods(cachedStatus); err != nil { + if err := d.resources.EnsurePods(cachedStatus); err != nil { return minInspectionInterval, errors.Wrapf(err, "Pod creation failed") } - if err := d.resources.EnsurePDBs(); err != nil { + if err := d.resources.EnsurePDBs(); err != nil { return minInspectionInterval, errors.Wrapf(err, "PDB creation failed") } - if err := d.resources.EnsureAnnotations(cachedStatus); err != nil { + if err := d.resources.EnsureAnnotations(cachedStatus); err != nil { return minInspectionInterval, errors.Wrapf(err, "Annotation update failed") } diff --git a/pkg/deployment/deployment_run_test.go b/pkg/deployment/deployment_run_test.go index 4dc68afb1..eaa58625c 100644 --- a/pkg/deployment/deployment_run_test.go +++ b/pkg/deployment/deployment_run_test.go @@ -25,9 +25,11 @@ package deployment import ( "encoding/json" "fmt" - "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" "testing" + "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + "github.com/arangodb/kube-arangodb/pkg/util/errors" + "github.com/arangodb/kube-arangodb/pkg/util" "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" core "k8s.io/api/core/v1" @@ -54,15 +56,34 @@ func runTestCase(t *testing.T, testCase testCaseStruct) { // Arrange d, eventRecorder := createTestDeployment(testCase.config, testCase.ArangoDeployment) - err := d.resources.EnsureSecrets(inspector.NewEmptyInspector()) - require.NoError(t, err) + errs := 0 + for { + cache, err := inspector.NewInspector(d.GetKubeCli(), d.GetNamespace()) + require.NoError(t, err) + err = d.resources.EnsureSecrets(cache) + if err == nil { + break + } + + if errs > 5 { + require.NoError(t, err) + } + + errs++ + + if errors.IsReconcile(err) { + continue + } + + require.NoError(t, err) + } if testCase.Helper != nil { testCase.Helper(t, d, &testCase) } // Create custom resource in the fake kubernetes API - _, err = d.deps.DatabaseCRCli.DatabaseV1().ArangoDeployments(testNamespace).Create(d.apiObject) + _, err := d.deps.DatabaseCRCli.DatabaseV1().ArangoDeployments(testNamespace).Create(d.apiObject) require.NoError(t, err) if testCase.Resources != nil { @@ -70,7 +91,9 @@ func runTestCase(t *testing.T, testCase testCaseStruct) { } // Act - err = d.resources.EnsurePods(inspector.NewEmptyInspector()) + cache, err := inspector.NewInspector(d.GetKubeCli(), d.GetNamespace()) + require.NoError(t, err) + err = d.resources.EnsurePods(cache) // Assert if testCase.ExpectedError != nil { diff --git a/pkg/deployment/reconcile/plan_builder.go b/pkg/deployment/reconcile/plan_builder.go index 42d75b8ff..70d0c9d20 100644 --- a/pkg/deployment/reconcile/plan_builder.go +++ b/pkg/deployment/reconcile/plan_builder.go @@ -115,7 +115,7 @@ func createPlan(ctx context.Context, log zerolog.Logger, apiObject k8sutil.APIOb } // Fetch agency plan - agencyPlan, agencyErr := fetchAgency(ctx, log, spec, status, builderCtx) + agencyPlan, agencyErr := fetchAgency(ctx, log, spec, status, builderCtx) // Check for various scenario's var plan api.Plan @@ -123,7 +123,7 @@ func createPlan(ctx context.Context, log zerolog.Logger, apiObject k8sutil.APIOb pb := NewWithPlanBuilder(ctx, log, apiObject, spec, status, cachedStatus, builderCtx) // Check for members in failed state - status.Members.ForeachServerGroup(func(group api.ServerGroup, members api.MemberStatusList) error { + status.Members.ForeachServerGroup(func(group api.ServerGroup, members api.MemberStatusList) error { for _, m := range members { if m.Phase != api.MemberPhaseFailed || len(plan) > 0 { continue @@ -180,7 +180,7 @@ func createPlan(ctx context.Context, log zerolog.Logger, apiObject k8sutil.APIOb } // Check for cleaned out dbserver in created state - for _, m := range status.Members.DBServers { + for _, m := range status.Members.DBServers { if plan.IsEmpty() && m.Phase.IsCreatedOrDrain() && m.Conditions.IsTrue(api.ConditionTypeCleanedOut) { log.Debug(). Str("id", m.ID). @@ -195,12 +195,12 @@ func createPlan(ctx context.Context, log zerolog.Logger, apiObject k8sutil.APIOb // Check for scale up/down if plan.IsEmpty() { - plan = pb.Apply(createScaleMemeberPlan) + plan = pb.Apply(createScaleMemeberPlan) } // Check for the need to rotate one or more members if plan.IsEmpty() { - plan = pb.Apply(createRotateOrUpgradePlan) + plan = pb.Apply(createRotateOrUpgradePlan) } // Add encryption keys diff --git a/pkg/deployment/reconcile/plan_builder_test.go b/pkg/deployment/reconcile/plan_builder_test.go index dca97d25d..abae76871 100644 --- a/pkg/deployment/reconcile/plan_builder_test.go +++ b/pkg/deployment/reconcile/plan_builder_test.go @@ -533,10 +533,10 @@ func TestCreatePlan(t *testing.T) { ExpectedLog string ExpectedEvent *k8sutil.Event - Pods map[string]*core.Pod - Secrets map[string]*core.Secret + Pods map[string]*core.Pod + Secrets map[string]*core.Secret Services map[string]*core.Service - PVCS map[string]*core.PersistentVolumeClaim + PVCS map[string]*core.PersistentVolumeClaim }{ { Name: "Can not create plan for single deployment", @@ -607,7 +607,7 @@ func TestCreatePlan(t *testing.T) { { Name: "Change Storage for Agents with deprecated storage class name", PVCS: map[string]*core.PersistentVolumeClaim{ - "pvc_test":{ + "pvc_test": { Spec: core.PersistentVolumeClaimSpec{ StorageClassName: util.NewString("oldStorage"), }, diff --git a/pkg/deployment/resources/inspector/inspector.go b/pkg/deployment/resources/inspector/inspector.go index 189e579cb..f0b2c261c 100644 --- a/pkg/deployment/resources/inspector/inspector.go +++ b/pkg/deployment/resources/inspector/inspector.go @@ -57,9 +57,9 @@ func NewEmptyInspector() Inspector { func NewInspectorFromData(pods map[string]*core.Pod, secrets map[string]*core.Secret, pvcs map[string]*core.PersistentVolumeClaim, services map[string]*core.Service) Inspector { return &inspector{ - pods: pods, - secrets: secrets, - pvcs: pvcs, + pods: pods, + secrets: secrets, + pvcs: pvcs, services: services, } } diff --git a/pkg/deployment/resources/secrets.go b/pkg/deployment/resources/secrets.go index 130114acd..acdd8b205 100644 --- a/pkg/deployment/resources/secrets.go +++ b/pkg/deployment/resources/secrets.go @@ -59,7 +59,7 @@ func (r *Resources) EnsureSecrets(cachedStatus inspector.Inspector) error { spec := r.context.GetSpec() kubecli := r.context.GetKubeCli() ns := r.context.GetNamespace() - secrets := k8sutil.NewSecretCache(kubecli.CoreV1().Secrets(ns)) + secrets := kubecli.CoreV1().Secrets(ns) status, _ := r.context.GetStatus() deploymentName := r.context.GetAPIObject().GetName() defer metrics.SetDuration(inspectSecretsDurationGauges.WithLabelValues(deploymentName), start) @@ -189,10 +189,10 @@ func AppendKeyfileToKeyfolder(cachedStatus inspector.Inspector, secrets k8sutil. } var ( - exporterTokenClaims = map[string]interface{}{ + exporterTokenClaims = jg.MapClaims{ "iss": "arangodb", "server_id": "exporter", - "allowed_paths": []string{"/_admin/statistics", "/_admin/statistics-description", k8sutil.ArangoExporterInternalEndpoint}, + "allowed_paths": []interface{}{"/_admin/statistics", "/_admin/statistics-description", k8sutil.ArangoExporterInternalEndpoint}, } ) @@ -235,12 +235,12 @@ func (r *Resources) ensureExporterTokenSecretCreateRequired(cachedStatus inspect jwtSecret, exists := cachedStatus.Secret(secretSecretName) if !exists { - return false, true, errors.Errorf("Secret %s does not exists", secretSecretName) + return true, true, errors.Errorf("Secret %s does not exists", secretSecretName) } secret, err := k8sutil.GetTokenFromSecret(jwtSecret) if err != nil { - return false, true, maskAny(err) + return true, true, maskAny(err) } token, err := jg.Parse(string(data), func(token *jg.Token) (i interface{}, err error) { @@ -256,7 +256,7 @@ func (r *Resources) ensureExporterTokenSecretCreateRequired(cachedStatus inspect return true, true, nil } - return !equality.Semantic.DeepEqual(tokenClaims, exporterTokenClaims), true, nil + return !equality.Semantic.DeepDerivative(tokenClaims, exporterTokenClaims), true, nil } } From dfc5e3dd28b0aa91b4d99e9a17abe533853be851 Mon Sep 17 00:00:00 2001 From: ajanikow <12255597+ajanikow@users.noreply.github.com> Date: Mon, 8 Jun 2020 11:25:57 +0000 Subject: [PATCH 3/3] Improve cluster removal --- pkg/deployment/cleanup.go | 29 ++++++++++++++++++------- pkg/deployment/deployment.go | 2 +- pkg/deployment/deployment_finalizers.go | 2 +- pkg/util/refs.go | 24 ++++++++++++++++++++ 4 files changed, 47 insertions(+), 10 deletions(-) diff --git a/pkg/deployment/cleanup.go b/pkg/deployment/cleanup.go index abe0939f4..3acaea6ee 100644 --- a/pkg/deployment/cleanup.go +++ b/pkg/deployment/cleanup.go @@ -24,8 +24,10 @@ package deployment import ( "github.com/arangodb/kube-arangodb/pkg/deployment/resources/inspector" + "github.com/arangodb/kube-arangodb/pkg/util" "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" core "k8s.io/api/core/v1" + meta "k8s.io/apimachinery/pkg/apis/meta/v1" ) // removePodFinalizers removes all finalizers from all pods owned by us. @@ -36,6 +38,16 @@ func (d *Deployment) removePodFinalizers(cachedStatus inspector.Inspector) error if err := cachedStatus.IteratePods(func(pod *core.Pod) error { if err := k8sutil.RemovePodFinalizers(log, kubecli, pod, pod.GetFinalizers(), true); err != nil { log.Warn().Err(err).Msg("Failed to remove pod finalizers") + return err + } + + if err := kubecli.CoreV1().Pods(pod.GetNamespace()).Delete(pod.GetName(), &meta.DeleteOptions{ + GracePeriodSeconds: util.NewInt64(1), + }); err != nil { + if !k8sutil.IsNotFound(err) { + log.Warn().Err(err).Msg("Failed to remove pod") + return err + } } return nil }, inspector.FilterPodsByLabels(k8sutil.LabelsForDeployment(d.GetName(), ""))); err != nil { @@ -46,18 +58,19 @@ func (d *Deployment) removePodFinalizers(cachedStatus inspector.Inspector) error } // removePVCFinalizers removes all finalizers from all PVCs owned by us. -func (d *Deployment) removePVCFinalizers() error { +func (d *Deployment) removePVCFinalizers(cachedStatus inspector.Inspector) error { log := d.deps.Log kubecli := d.GetKubeCli() - pvcs, err := d.GetOwnedPVCs() - if err != nil { - return maskAny(err) - } - for _, p := range pvcs { - ignoreNotFound := true - if err := k8sutil.RemovePVCFinalizers(log, kubecli, &p, p.GetFinalizers(), ignoreNotFound); err != nil { + + if err := cachedStatus.IteratePersistentVolumeClaims(func(pvc *core.PersistentVolumeClaim) error { + if err := k8sutil.RemovePVCFinalizers(log, kubecli, pvc, pvc.GetFinalizers(), true); err != nil { log.Warn().Err(err).Msg("Failed to remove PVC finalizers") + return err } + return nil + }, inspector.FilterPersistentVolumeClaimsByLabels(k8sutil.LabelsForDeployment(d.GetName(), ""))); err != nil { + return err } + return nil } diff --git a/pkg/deployment/deployment.go b/pkg/deployment/deployment.go index b25d56611..d5009bc04 100644 --- a/pkg/deployment/deployment.go +++ b/pkg/deployment/deployment.go @@ -241,7 +241,7 @@ func (d *Deployment) run() { if err := d.removePodFinalizers(cachedStatus); err != nil { log.Warn().Err(err).Msg("Failed to remove Pod finalizers") } - if err := d.removePVCFinalizers(); err != nil { + if err := d.removePVCFinalizers(cachedStatus); err != nil { log.Warn().Err(err).Msg("Failed to remove PVC finalizers") } // We're being stopped. diff --git a/pkg/deployment/deployment_finalizers.go b/pkg/deployment/deployment_finalizers.go index cae2d6c14..c631aaa0d 100644 --- a/pkg/deployment/deployment_finalizers.go +++ b/pkg/deployment/deployment_finalizers.go @@ -85,7 +85,7 @@ func (d *Deployment) inspectRemoveChildFinalizers(ctx context.Context, log zerol if err := d.removePodFinalizers(cachedStatus); err != nil { return maskAny(err) } - if err := d.removePVCFinalizers(); err != nil { + if err := d.removePVCFinalizers(cachedStatus); err != nil { return maskAny(err) } diff --git a/pkg/util/refs.go b/pkg/util/refs.go index dd1c4142f..2f364a0cd 100644 --- a/pkg/util/refs.go +++ b/pkg/util/refs.go @@ -100,6 +100,30 @@ func Int32OrDefault(input *int32, defaultValue ...int32) int32 { return *input } +// NewInt64 returns a reference to an int64 with given value. +func NewInt64(input int64) *int64 { + return &input +} + +// NewInt64OrNil returns nil if input is nil, otherwise returns a clone of the given value. +func NewInt64OrNil(input *int64) *int64 { + if input == nil { + return nil + } + return NewInt64(*input) +} + +// Int64OrDefault returns the default value (or 0) if input is nil, otherwise returns the referenced value. +func Int64OrDefault(input *int64, defaultValue ...int64) int64 { + if input == nil { + if len(defaultValue) > 0 { + return defaultValue[0] + } + return 0 + } + return *input +} + // NewUInt16 returns a reference to an uint16 with given value. func NewUInt16(input uint16) *uint16 { return &input