Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cluster: delete PVCs after decom if flag is set #206

Merged
merged 2 commits into from
Sep 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions .buildkite/pipeline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,70 @@ steps:
failed: true
branches:
- main
- key: k8s-operator-with-flags
label: K8s Operator tests with flags
timeout_in_minutes: 180
notify:
- github_commit_status:
context: k8s-operator
commands:
- |
TAG_NAME=$(ci/scripts/tag-check.sh) ./ci/scripts/run-in-nix-docker.sh ./task ci:run-k8s-tests-with-flags
agents:
queue: amd64-builders
artifact_paths:
- src/go/k8s/*.tar.gz
- src/go/k8s/tests/_e2e_artifacts/kuttl-report.xml
plugins:
- seek-oss/aws-sm#v2.3.2: &aws-sm-plugin
json-to-env:
- json-key: .
secret-id: sdlc/prod/buildkite/active_directory
- json-key: .
secret-id: sdlc/prod/buildkite/buildkite_analytics_token
- json-key: .
secret-id: sdlc/prod/buildkite/buildkite_api_token
- json-key: .
secret-id: sdlc/prod/buildkite/cdt_gcp
- json-key: .
secret-id: sdlc/prod/buildkite/cdt_runner_aws
- json-key: .
secret-id: sdlc/prod/buildkite/ci_db
- json-key: .
secret-id: sdlc/prod/buildkite/cloudsmith
- json-key: .
secret-id: sdlc/prod/buildkite/dockerhub
- json-key: .
secret-id: sdlc/prod/buildkite/gh_token
- json-key: .
secret-id: sdlc/prod/buildkite/github_api_token
- json-key: .
secret-id: sdlc/prod/buildkite/goreleaser_key
- json-key: .
secret-id: sdlc/prod/buildkite/grafana_token
- json-key: .
secret-id: sdlc/prod/buildkite/redpanda_sample_license
- json-key: .
secret-id: sdlc/prod/buildkite/redpanda_second_sample_license
- json-key: .
secret-id: sdlc/prod/buildkite/rpk_test_client
- json-key: .
secret-id: sdlc/prod/buildkite/seceng_audit_aws
- json-key: .
secret-id: sdlc/prod/buildkite/slack_vbot_token
- json-key: .
secret-id: sdlc/prod/buildkite/teleport_bot_token
- json-key: .
secret-id: sdlc/prod/buildkite/test_result_dsn
- https://[email protected]/redpanda-data/step-slack-notify-buildkite-plugin.git#main:
message: ":cloud: K8s Operator v1 Jobs failed"
channel_name: "kubernetes-tests"
slack_token_env_var_name: "SLACK_VBOT_TOKEN"
conditions:
failed: true
branches:
- main


- group: K8s Operator v2 Jobs
if: |
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
/dbuild
bin
_e2e_artifacts/
_e2e_with_flags_artifacts/
_e2e_unstable_artifacts/
_helm_e2e_artifacts/
src/go/k8s/testbin/*
Expand Down
3 changes: 3 additions & 0 deletions src/go/k8s/cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ func main() {
debug bool
ghostbuster bool
unbindPVCsAfter time.Duration
autoDeletePVCs bool
)

flag.StringVar(&eventsAddr, "events-addr", "", "The address of the events receiver.")
Expand Down Expand Up @@ -180,6 +181,7 @@ func main() {
flag.BoolVar(&operatorMode, "operator-mode", true, "enables to run as an operator, setting this to false will disable cluster (deprecated), redpanda resources reconciliation.")
flag.BoolVar(&enableHelmControllers, "enable-helm-controllers", true, "if a namespace is defined and operator mode is true, this enables the use of helm controllers to manage fluxcd helm resources.")
flag.DurationVar(&unbindPVCsAfter, "unbind-pvcs-after", 0, "if not zero, runs the PVCUnbinder controller which attempts to 'unbind' the PVCs' of Pods that are Pending for longer than the given duration")
flag.BoolVar(&autoDeletePVCs, "auto-delete-pvcs", false, "Use StatefulSet PersistentVolumeClaimRetentionPolicy to auto delete PVCs on scale down and Cluster resource delete.")

logOptions.BindFlags(flag.CommandLine)
clientOptions.BindFlags(flag.CommandLine)
Expand Down Expand Up @@ -265,6 +267,7 @@ func main() {
MetricsTimeout: metricsTimeout,
RestrictToRedpandaVersion: restrictToRedpandaVersion,
GhostDecommissioning: ghostbuster,
AutoDeletePVCs: autoDeletePVCs,
}).WithClusterDomain(clusterDomain).WithConfiguratorSettings(configurator).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "Unable to create controller", "controller", "Cluster")
os.Exit(1)
Expand Down
15 changes: 15 additions & 0 deletions src/go/k8s/config/e2e-tests-with-flags/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- ../e2e-tests
patches:
- patch: |-
- op: add
path: /spec/template/spec/containers/0/args/-
value: --auto-delete-pvcs
target:
group: apps
version: v1
kind: Deployment
name: controller-manager
2 changes: 1 addition & 1 deletion src/go/k8s/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ require (
k8s.io/apimachinery v0.29.5
k8s.io/client-go v0.29.5
k8s.io/component-helpers v0.29.0
k8s.io/kubectl v0.29.0
k8s.io/utils v0.0.0-20240310230437-4693a0247e57
pgregory.net/rapid v1.1.0
sigs.k8s.io/controller-runtime v0.17.2
Expand Down Expand Up @@ -407,7 +408,6 @@ require (
k8s.io/component-base v0.29.5 // indirect
k8s.io/klog/v2 v2.120.1 // indirect
k8s.io/kube-openapi v0.0.0-20240103051144-eec4567ac022 // indirect
k8s.io/kubectl v0.29.0 // indirect
oras.land/oras-go v1.2.5 // indirect
sigs.k8s.io/gateway-api v1.0.0 // indirect
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
Expand Down
23 changes: 7 additions & 16 deletions src/go/k8s/internal/controller/redpanda/cluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import (
adminutils "github.com/redpanda-data/redpanda-operator/src/go/k8s/pkg/admin"
"github.com/redpanda-data/redpanda-operator/src/go/k8s/pkg/labels"
"github.com/redpanda-data/redpanda-operator/src/go/k8s/pkg/networking"
"github.com/redpanda-data/redpanda-operator/src/go/k8s/pkg/patch"
"github.com/redpanda-data/redpanda-operator/src/go/k8s/pkg/resources"
"github.com/redpanda-data/redpanda-operator/src/go/k8s/pkg/resources/featuregates"
"github.com/redpanda-data/redpanda-operator/src/go/k8s/pkg/utils"
Expand Down Expand Up @@ -75,6 +76,7 @@ type ClusterReconciler struct {
MetricsTimeout time.Duration
RestrictToRedpandaVersion string
GhostDecommissioning bool
AutoDeletePVCs bool
}

//+kubebuilder:rbac:groups=apps,resources=statefulsets,verbs=get;list;watch;create;update;patch;delete
Expand Down Expand Up @@ -140,7 +142,7 @@ func (r *ClusterReconciler) Reconcile(
// - Set OperatorQuiescent condition, based on our best knowledge if there is
// any outstanding work to do for the controller.
defer func() {
_, patchErr := patchStatus(ctx, r.Client, &vectorizedCluster, func(cluster *vectorizedv1alpha1.Cluster) {
_, patchErr := patch.PatchStatus(ctx, r.Client, &vectorizedCluster, func(cluster *vectorizedv1alpha1.Cluster) {
// Set quiescent
cond := getQuiescentCondition(cluster)

Expand Down Expand Up @@ -360,9 +362,9 @@ func (r *ClusterReconciler) removePodFinalizer(
log := l.WithName("removePodFinalizer")
if controllerutil.ContainsFinalizer(pod, FinalizerKey) {
log.V(logger.DebugLevel).WithValues("namespace", pod.Namespace, "name", pod.Name).Info("removing finalizer")
patch := client.MergeFrom(pod.DeepCopy())
p := client.MergeFrom(pod.DeepCopy())
controllerutil.RemoveFinalizer(pod, FinalizerKey)
if err := r.Patch(ctx, pod, patch); err != nil {
if err := r.Patch(ctx, pod, p); err != nil {
return fmt.Errorf("unable to remove pod (%s/%s) finalizer: %w", pod.Namespace, pod.Name, err)
}
}
Expand Down Expand Up @@ -764,9 +766,9 @@ func (r *ClusterReconciler) removeFinalizers(

if controllerutil.ContainsFinalizer(redpandaCluster, FinalizerKey) {
log.V(logger.DebugLevel).Info("removing finalizers from cluster custom resource")
patch := client.MergeFrom(redpandaCluster.DeepCopy())
p := client.MergeFrom(redpandaCluster.DeepCopy())
controllerutil.RemoveFinalizer(redpandaCluster, FinalizerKey)
if err := r.Patch(ctx, redpandaCluster, patch); err != nil {
if err := r.Patch(ctx, redpandaCluster, p); err != nil {
return fmt.Errorf("unable to remove Cluster finalizer: %w", err)
}
}
Expand Down Expand Up @@ -1130,17 +1132,6 @@ func isRedpandaClusterVersionManaged(
return true
}

func patchStatus(ctx context.Context, c client.Client, observedCluster *vectorizedv1alpha1.Cluster, mutator func(cluster *vectorizedv1alpha1.Cluster)) (vectorizedv1alpha1.ClusterStatus, error) {
clusterPatch := client.MergeFrom(observedCluster.DeepCopy())
mutator(observedCluster)

if err := c.Status().Patch(ctx, observedCluster, clusterPatch); err != nil {
return vectorizedv1alpha1.ClusterStatus{}, fmt.Errorf("failed to update cluster status: %w", err)
}

return observedCluster.Status, nil
}

func getQuiescentCondition(redpandaCluster *vectorizedv1alpha1.Cluster) vectorizedv1alpha1.ClusterCondition {
condition := vectorizedv1alpha1.ClusterCondition{
Type: vectorizedv1alpha1.OperatorQuiescentConditionType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@ import (
)

type attachedResources struct {
ctx context.Context
reconciler *ClusterReconciler
log logr.Logger
cluster *vectorizedv1alpha1.Cluster
items map[string]resources.Resource
order []string
ctx context.Context
reconciler *ClusterReconciler
log logr.Logger
cluster *vectorizedv1alpha1.Cluster
items map[string]resources.Resource
order []string
autoDeletePVCs bool
}

const (
Expand All @@ -43,11 +44,12 @@ const (

func newAttachedResources(ctx context.Context, r *ClusterReconciler, log logr.Logger, cluster *vectorizedv1alpha1.Cluster) *attachedResources {
return &attachedResources{
ctx: ctx,
reconciler: r,
log: log,
cluster: cluster,
items: map[string]resources.Resource{},
ctx: ctx,
reconciler: r,
log: log,
cluster: cluster,
items: map[string]resources.Resource{},
autoDeletePVCs: r.AutoDeletePVCs,
}
}

Expand Down Expand Up @@ -393,7 +395,8 @@ func (a *attachedResources) statefulSet() error {
a.reconciler.AdminAPIClientFactory,
a.reconciler.DecommissionWaitInterval,
a.log,
a.reconciler.MetricsTimeout)
a.reconciler.MetricsTimeout,
a.autoDeletePVCs)
a.order = append(a.order, statefulSet)
return nil
}
Expand Down
12 changes: 12 additions & 0 deletions src/go/k8s/kind-for-cloud.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
kind: Cluster
apiVersion: kind.x-k8s.io/v1alpha4
nodes:
- role: control-plane
# Need to run KIND 0.19 based image; >0.19 does not support cgroupsv1/missing cgroupns - and this is required for CI at the moment.
image: kindest/node:v1.28.0@sha256:dad5a6238c5e41d7cac405fae3b5eda2ad1de6f1190fa8bfc64ff5bb86173213
- role: worker
image: kindest/node:v1.28.0@sha256:dad5a6238c5e41d7cac405fae3b5eda2ad1de6f1190fa8bfc64ff5bb86173213
- role: worker
image: kindest/node:v1.28.0@sha256:dad5a6238c5e41d7cac405fae3b5eda2ad1de6f1190fa8bfc64ff5bb86173213
- role: worker
image: kindest/node:v1.28.0@sha256:dad5a6238c5e41d7cac405fae3b5eda2ad1de6f1190fa8bfc64ff5bb86173213
30 changes: 30 additions & 0 deletions src/go/k8s/kuttl-test-with-flags.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
apiVersion: kuttl.dev/v1beta1
kind: TestSuite
startKIND: true
kindContainers:
- localhost/redpanda-operator:dev
- localhost/configurator:dev
- localhost/redpanda:dev
testDirs:
- ./tests/e2e-with-flags
kindConfig: ./kind-for-cloud.yaml
kindNodeCache: false
commands:
- command: kubectl taint node kind-control-plane node-role.kubernetes.io/control-plane-
- command: "mkdir -p tests/_e2e_with_flags_artifacts"
- command: "./hack/install-cert-manager.sh tests/_e2e_with_flags_artifacts"
background: true
ignoreFailure: true
- command: "kubectl create -f https://raw.githubusercontent.com/prometheus-operator/prometheus-operator/e23ff77fceba6a5d9f190f5d1a123c87701dc964/bundle.yaml"
background: true
ignoreFailure: true
- command: "sh -c 'until kustomize build config/e2e-tests-with-flags 2>> tests/_e2e_with_flags_artifacts/kustomize-output.txt | kubectl apply --server-side -f - 1>> tests/_e2e_with_flags_artifacts/kubectl-output.txt 2>> tests/_e2e_with_flags_artifacts/kubectl-error-output.txt; do sleep 0.5; done'"
background: true
- command: "./hack/wait-for-webhook-ready.sh"
artifactsDir: tests/_e2e_with_flags_artifacts
timeout: 330
reportFormat: xml
parallel: 2
namespace: redpanda-system
suppress:
- events
25 changes: 25 additions & 0 deletions src/go/k8s/pkg/patch/patch.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Package patch is a utility package that provides utils around patching a resource.
// It has its own package, because of a dependency conflict; pkg/utils may not
// import types/v1alpha1, types/v1alpha1 imports pkg/utils (cycle).
package patch

import (
"context"
"fmt"

vectorizedv1alpha1 "github.com/redpanda-data/redpanda-operator/src/go/k8s/api/vectorized/v1alpha1"
"sigs.k8s.io/controller-runtime/pkg/client"
)

// PatchStatus persforms a mutation as done by mutator, calls k8s-api with PATCH, and then returns the
// new status.
func PatchStatus(ctx context.Context, c client.Client, observedCluster *vectorizedv1alpha1.Cluster, mutator func(cluster *vectorizedv1alpha1.Cluster)) (vectorizedv1alpha1.ClusterStatus, error) {
clusterPatch := client.MergeFrom(observedCluster.DeepCopy())
mutator(observedCluster)

if err := c.Status().Patch(ctx, observedCluster, clusterPatch); err != nil {
return vectorizedv1alpha1.ClusterStatus{}, fmt.Errorf("failed to update cluster status: %w", err)
}

return observedCluster.Status, nil
}
4 changes: 3 additions & 1 deletion src/go/k8s/pkg/resources/resource_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@ func TestEnsure_StatefulSet(t *testing.T) {
adminutils.NewInternalAdminAPI,
time.Second,
ctrl.Log.WithName("test"),
0)
0,
true,
)

err = sts.Ensure(context.Background())
assert.NoError(t, err)
Expand Down
24 changes: 21 additions & 3 deletions src/go/k8s/pkg/resources/statefulset.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ const (
defaultDatadirCapacity = "100Gi"
trueString = "true"

// PodAnnotationNodeIDKey is identical to its label counterpart.
PodAnnotationNodeIDKey = "operator.redpanda.com/node-id"
)

Expand Down Expand Up @@ -115,6 +116,8 @@ type StatefulSetResource struct {
metricsTimeout time.Duration

LastObservedState *appsv1.StatefulSet

autoDeletePVCs bool
}

// NewStatefulSet creates StatefulSetResource
Expand All @@ -134,6 +137,7 @@ func NewStatefulSet(
decommissionWaitInterval time.Duration,
logger logr.Logger,
metricsTimeout time.Duration,
autoDeletePVCs bool,
) *StatefulSetResource {
ssr := &StatefulSetResource{
client,
Expand All @@ -153,6 +157,7 @@ func NewStatefulSet(
logger.WithName("StatefulSetResource"),
defaultAdminAPITimeout,
nil,
autoDeletePVCs,
}
if metricsTimeout != 0 {
ssr.metricsTimeout = metricsTimeout
Expand Down Expand Up @@ -338,6 +343,18 @@ func (r *StatefulSetResource) obj(
fmt.Sprintf("--admin-api-tls-key %q", path.Join(resourcetypes.GetTLSMountPoints().AdminAPI.ClientCAMountDir, "tls.key")))
}

// In any case, configure PersistentVolumeClaimRetentionPolicy
// Default to old behavior: Retain PVC
// If auto-remove-pvcs flag is set, active new behavior: switch to Delete for both WhenScaled and WhenDeleted.
var pvcReclaimRetentionPolicy appsv1.StatefulSetPersistentVolumeClaimRetentionPolicy
if r.autoDeletePVCs {
pvcReclaimRetentionPolicy.WhenScaled = appsv1.DeletePersistentVolumeClaimRetentionPolicyType
pvcReclaimRetentionPolicy.WhenDeleted = appsv1.DeletePersistentVolumeClaimRetentionPolicyType
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The paranoia in me says we may want to keep when deleted as retain? I can't really imagine someone scaling the cluster manually but I could see cases where the STS needs to be deleted with orphan = true to work around it's immutability.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh you're so right. GOOD CALL!
Delete on orphan DOES happen, and is actively used by the operator. Will change When deleted to RETAIN.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually need to test if orphan delete triggers this behavior..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks like deleting with orphan does not get PVCs deleted. obviously, since pods are retained as well. i can add a test. then it's fine to keep using delete on delete, what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

switched to Patch call. moved the patch helper func from cluster_controller to pkg/patch (pkg/utils does not work because of import conflict w/ types package)

} else {
pvcReclaimRetentionPolicy.WhenScaled = appsv1.RetainPersistentVolumeClaimRetentionPolicyType
pvcReclaimRetentionPolicy.WhenDeleted = appsv1.RetainPersistentVolumeClaimRetentionPolicyType
}

// We set statefulset replicas via status.currentReplicas in order to control it from the handleScaling function
replicas := r.pandaCluster.GetCurrentReplicas()
ss := &appsv1.StatefulSet{
Expand All @@ -351,9 +368,10 @@ func (r *StatefulSetResource) obj(
APIVersion: "apps/v1",
},
Spec: appsv1.StatefulSetSpec{
Replicas: &replicas,
PodManagementPolicy: appsv1.ParallelPodManagement,
Selector: clusterLabels.AsAPISelector(),
PersistentVolumeClaimRetentionPolicy: &pvcReclaimRetentionPolicy,
Replicas: &replicas,
PodManagementPolicy: appsv1.ParallelPodManagement,
Selector: clusterLabels.AsAPISelector(),
UpdateStrategy: appsv1.StatefulSetUpdateStrategy{
Type: appsv1.OnDeleteStatefulSetStrategyType,
},
Expand Down
Loading