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

Support restart task for StatefulSet with updateStrategy: OnDelete #840

Merged
merged 7 commits into from
Feb 2, 2022
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
## next

*Enhancements*

- Support restart task for stateful sets that use the `OnDelete` strategy [#840](https://github.com/Shopify/krane/pull/840)

## 2.4.0

*Enhancements*
Expand Down
10 changes: 4 additions & 6 deletions lib/krane/kubernetes_resource/stateful_set.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,11 @@ def deploy_succeeded?
"Consider switching to rollingUpdate.")
@success_assumption_warning_shown = true
end
true
else
observed_generation == current_generation &&
status_data['currentRevision'] == status_data['updateRevision'] &&
desired_replicas == status_data['readyReplicas'].to_i &&
desired_replicas == status_data['currentReplicas'].to_i
end
observed_generation == current_generation &&
status_data['currentRevision'] == status_data['updateRevision'] &&
desired_replicas == status_data['readyReplicas'].to_i &&
desired_replicas == status_data['currentReplicas'].to_i
end

def deploy_failed?
Expand Down
35 changes: 24 additions & 11 deletions lib/krane/restart_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ def run!(deployments: [], statefulsets: [], daemonsets: [], selector: nil, verif
deployments, statefulsets, daemonsets = identify_target_workloads(deployments, statefulsets,
daemonsets, selector: selector)

@logger.phase_heading("Triggering restart by annotating pod template #{RESTART_TRIGGER_ANNOTATION} annotation")
patch_kubeclient_deployments(deployments)
patch_kubeclient_statefulsets(statefulsets)
patch_kubeclient_daemonsets(daemonsets)
@logger.phase_heading("Triggering restart")
restart_deployments!(deployments)
restart_statefulsets!(statefulsets)
restart_daemonsets!(daemonsets)

if verify_result
@logger.phase_heading("Waiting for rollout")
Expand Down Expand Up @@ -210,7 +210,15 @@ def patch_daemonset_with_restart(record)
apps_v1_kubeclient.patch_daemon_set(record.metadata.name, build_patch_payload(record), @namespace)
end

def patch_kubeclient_deployments(deployments)
def delete_statefulset_pods(record)
pods = kubeclient.get_pods(namespace: record.metadata.namespace)
pods.select! do |pod|
pod.metadata.ownerReferences.find { |ref| ref.uid == record.metadata.uid }

Choose a reason for hiding this comment

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

is record.metadata.uid something that identifies individual statefulsets?

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 uniquely identifies the resource throughout the lifetime of the cluster. That is, no 2 resources in a cluster will ever have the same UID

end
pods.each { |pod| kubeclient.delete_pod(pod.metadata.name, pod.metadata.namespace) }
end

def restart_deployments!(deployments)
deployments.each do |record|
begin
patch_deployment_with_restart(record)
Expand All @@ -221,18 +229,23 @@ def patch_kubeclient_deployments(deployments)
end
end

def patch_kubeclient_statefulsets(statefulsets)
def restart_statefulsets!(statefulsets)
statefulsets.each do |record|
begin
@logger.info("Triggered `StatefulSet/#{record.metadata.name}` restart")
if record.spec.updateStrategy&.type == "OnDelete"
@logger.info("`StatefulSet/#{record.metadata.name}` has updateStrategy: OnDelete," \
" Restarting by forcefully deleting child pods"
)
delete_statefulset_pods(record)
else
patch_statefulset_with_restart(record)
@logger.info("Triggered `StatefulSet/#{record.metadata.name}` restart")
rescue Kubeclient::HttpError => e
raise RestartAPIError.new(record.metadata.name, e.message)
end
rescue Kubeclient::HttpError => e
raise RestartAPIError.new(record.metadata.name, e.message)
end
end

def patch_kubeclient_daemonsets(daemonsets)
def restart_daemonsets!(daemonsets)
daemonsets.each do |record|
begin
patch_daemonset_with_restart(record)
Expand Down
29 changes: 29 additions & 0 deletions test/integration/restart_task_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,35 @@ def test_restart_by_annotation
refute(fetch_restarted_at("redis"), "no restart annotation env on fresh deployment")
end

def test_restart_statefulset_on_delete_restarts_child_pods
result = deploy_fixtures("hello-cloud", subset: "stateful_set.yml") do |fixtures|
statefulset = fixtures["stateful_set.yml"]["StatefulSet"].first
statefulset["spec"]["updateStrategy"] = { "type" => "OnDelete" }
end
assert_deploy_success(result)
before_pods = kubeclient.get_pods(namespace: @namespace, label_selector: "name=stateful-busybox").map do |p|
p.metadata.uid
end

restart = build_restart_task
assert_restart_success(restart.perform)
after_pods = kubeclient.get_pods(namespace: @namespace, label_selector: "name=stateful-busybox").map do |p|
p.metadata.uid
end
refute_equal(before_pods.sort, after_pods.sort)

assert_logs_match_all([
"Configured to restart all workloads with the `shipit.shopify.io/restart` annotation",
"Triggered `StatefulSet/stateful-busybox` restart",
"`StatefulSet/stateful-busybox` has updateStrategy: OnDelete, Restarting by forcefully deleting child pods",
"Waiting for rollout",
"Result: SUCCESS",
"Successfully restarted 1 resource",
%r{StatefulSet/stateful-busybox.* 2 replicas},
],
in_order: true)
end

def test_restart_by_selector
assert_deploy_success(deploy_fixtures("branched",
bindings: { "branch" => "master" },
Expand Down