Support restart task for statefulsets and daemonsets #836
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,8 @@ | ||
| inherit_gem: | ||
| rubocop-shopify: rubocop.yml | ||
|
|
||
| Style/DateTime: | ||
| Enabled: false | ||
|
|
||
| AllCops: | ||
| TargetRubyVersion: 2.4 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,8 @@ def initialize(deployment_name, response) | |
| HTTP_OK_RANGE = 200..299 | ||
| ANNOTATION = "shipit.shopify.io/restart" | ||
|
|
||
| RESTART_TRIGGER_ANNOTATION = "kubectl.kubernetes.io/restartedAt" | ||
|
|
||
| attr_reader :task_config | ||
|
|
||
| delegate :kubeclient_builder, to: :task_config | ||
|
|
@@ -58,20 +60,25 @@ def run(**args) | |
| # @param verify_result [Boolean] Wait for completion and verify success | ||
| # | ||
| # @return [nil] | ||
| def run!(deployments: [], statefulsets: [], daemonsets: [], selector: nil, verify_result: true) | ||
| start = Time.now.utc | ||
| @logger.reset | ||
|
|
||
| @logger.phase_heading("Initializing restart") | ||
| verify_config! | ||
| deployments, statefulsets, daemonsets = identify_target_workloads(deployments, statefulsets, | ||
| daemonsets, selector: selector) | ||
|
|
||
| @logger.phase_heading("Triggering restart by touching ENV[RESTARTED_AT]") | ||
| patch_kubeclient_deployments(deployments) | ||
| patch_kubeclient_statefulsets(statefulsets) | ||
| patch_kubeclient_daemonsets(daemonsets) | ||
|
|
||
| if verify_result | ||
| @logger.phase_heading("Waiting for rollout") | ||
| resources = build_watchables(deployments, start, Deployment) | ||
| resources += build_watchables(statefulsets, start, StatefulSet) | ||
| resources += build_watchables(daemonsets, start, DaemonSet) | ||
| verify_restart(resources) | ||
| else | ||
| warning = "Result verification is disabled for this task" | ||
|
|
@@ -97,44 +104,93 @@ def tags(status, deployments) | |
| %W(namespace:#{@namespace} context:#{@context} status:#{status} deployments:#{deployments.to_a.length}}) | ||
| end | ||
|
|
||
| def identify_target_workloads(deployment_names, statefulset_names, daemonset_names, selector: nil) | ||
| if deployment_names.blank? && statefulset_names.blank? && daemonset_names.blank? | ||
|
There was a problem hiding this comment. Curious why There was a problem hiding this comment. While the default arg is the empty list, I can't guarantee that someone won't explicitly pass |
||
| if selector.nil? | ||
| @logger.info("Configured to restart all workloads with the `#{ANNOTATION}` annotation") | ||
| else | ||
| @logger.info( | ||
| "Configured to restart all workloads with the `#{ANNOTATION}` annotation and #{selector} selector" | ||
| ) | ||
| end | ||
| deployments = identify_target_deployments(selector: selector) | ||
| statefulsets = identify_target_statefulsets(selector: selector) | ||
| daemonsets = identify_target_daemonsets(selector: selector) | ||
|
|
||
| if deployments.none? && statefulsets.none? && daemonsets.none? | ||
| raise FatalRestartError, "no deployments, statefulsets, or daemonsets, with the `#{ANNOTATION}` " \ | ||
| "annotation found in namespace #{@namespace}" | ||
| end | ||
| elsif deployment_names.empty? && statefulset_names.empty? && daemonset_names.empty? | ||
| raise FatalRestartError, "Configured to restart workloads by name, but list of names was blank" | ||
| elsif !selector.nil? | ||
| raise FatalRestartError, "Can't specify workload names and selector at the same time" | ||
| else | ||
| deployments, statefulsets, daemonsets = identify_target_workloads_by_name(deployment_names, | ||
| statefulset_names, daemonset_names) | ||
| if deployments.none? && statefulsets.none? && daemonsets.none? | ||
| error_msgs = [] | ||
| error_msgs << "no deployments with names #{list} found in namespace #{@namespace}" if deployment_names | ||
| error_msgs << "no statefulsets with names #{list} found in namespace #{@namespace}" if statefulset_names | ||
| error_msgs << "no daemonsets with names #{list} found in namespace #{@namespace}" if daemonset_names | ||
| raise FatalRestartError, error_msgs.join(', ') | ||
| end | ||
| end | ||
| [deployments, statefulsets, daemonsets] | ||
| end | ||
|
|
||
| def identify_target_workloads_by_name(deployment_names, statefulset_names, daemonset_names) | ||
| deployment_names = deployment_names.uniq | ||
| statefulset_names = statefulset_names.uniq | ||
| daemonset_names = daemonset_names.uniq | ||
|
|
||
| if deployment_names.present? | ||
| @logger.info("Configured to restart deployments by name: #{deployment_names.join(', ')}") | ||
| end | ||
| if statefulset_names.present? | ||
| @logger.info("Configured to restart statefulsets by name: #{statefulset_names.join(', ')}") | ||
| end | ||
| if daemonset_names.present? | ||
| @logger.info("Configured to restart daemonsets by name: #{daemonset_names.join(', ')}") | ||
| end | ||
|
|
||
| [fetch_deployments(deployment_names), fetch_statefulsets(statefulset_names), fetch_daemonsets(daemonset_names)] | ||
| end | ||
|
|
||
| def identify_target_deployments(selector: nil) | ||
| deployments = if selector.nil? | ||
| apps_v1_kubeclient.get_deployments(namespace: @namespace) | ||
| else | ||
| selector_string = selector.to_s | ||
| apps_v1_kubeclient.get_deployments(namespace: @namespace, label_selector: selector_string) | ||
| end | ||
| deployments.select { |d| d.metadata.annotations[ANNOTATION] } | ||
| end | ||
|
|
||
| def identify_target_statefulsets(selector: nil) | ||
| statefulsets = if selector.nil? | ||
| apps_v1_kubeclient.get_stateful_sets(namespace: @namespace) | ||
| else | ||
| selector_string = selector.to_s | ||
| apps_v1_kubeclient.get_stateful_sets(namespace: @namespace, label_selector: selector_string) | ||
| end | ||
| statefulsets.select { |d| d.metadata.annotations[ANNOTATION] } | ||
| end | ||
|
|
||
| def identify_target_daemonsets(selector: nil) | ||
| daemonsets = if selector.nil? | ||
| apps_v1_kubeclient.get_daemon_sets(namespace: @namespace) | ||
| else | ||
| selector_string = selector.to_s | ||
| apps_v1_kubeclient.get_daemon_sets(namespace: @namespace, label_selector: selector_string) | ||
| end | ||
| daemonsets.select { |d| d.metadata.annotations[ANNOTATION] } | ||
| end | ||
|
|
||
| def build_watchables(kubeclient_resources, started, klass) | ||
| kubeclient_resources.map do |d| | ||
| definition = d.to_h.deep_stringify_keys | ||
| r = klass.new(namespace: @namespace, context: @context, definition: definition, logger: @logger) | ||
| r.deploy_started_at = started # we don't care what happened to the resource before the restart cmd ran | ||
| r | ||
| end | ||
|
|
@@ -148,11 +204,49 @@ def patch_deployment_with_restart(record) | |
| ) | ||
| end | ||
|
|
||
| def patch_statefulset_with_restart(record) | ||
| apps_v1_kubeclient.patch_stateful_set( | ||
| record.metadata.name, | ||
| build_patch_payload(record), | ||
| @namespace | ||
| ) | ||
| end | ||
|
|
||
| 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) | ||
| deployments.each do |record| | ||
| begin | ||
| patch_deployment_with_restart(record) | ||
| @logger.info("Triggered `Deployment/#{record.metadata.name}` restart") | ||
| rescue Kubeclient::HttpError => e | ||
| raise RestartAPIError.new(record.metadata.name, e.message) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| def patch_kubeclient_statefulsets(statefulsets) | ||
| statefulsets.each do |record| | ||
| begin | ||
| 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 | ||
| end | ||
| end | ||
|
|
||
| def patch_kubeclient_daemonsets(daemonsets) | ||
| daemonsets.each do |record| | ||
| begin | ||
| patch_daemonset_with_restart(record) | ||
| @logger.info("Triggered `DaemonSet/#{record.metadata.name}` restart") | ||
| rescue Kubeclient::HttpError => e | ||
| raise RestartAPIError.new(record.metadata.name, e.message) | ||
| end | ||
|
|
@@ -171,18 +265,38 @@ def fetch_deployments(list) | |
| end | ||
| end | ||
|
|
||
| def fetch_statefulsets(list) | ||
| list.map do |name| | ||
| record = nil | ||
| begin | ||
| record = apps_v1_kubeclient.get_stateful_set(name, @namespace) | ||
| rescue Kubeclient::ResourceNotFoundError | ||
| raise FatalRestartError, "StatefulSet `#{name}` not found in namespace `#{@namespace}`" | ||
| end | ||
| record | ||
| end | ||
| end | ||
|
|
||
| def fetch_daemonsets(list) | ||
| list.map do |name| | ||
| record = nil | ||
| begin | ||
| record = apps_v1_kubeclient.get_daemon_set(name, @namespace) | ||
| rescue Kubeclient::ResourceNotFoundError | ||
| raise FatalRestartError, "DaemonSet `#{name}` not found in namespace `#{@namespace}`" | ||
| end | ||
| record | ||
| end | ||
| end | ||
|
|
||
| def build_patch_payload(_deployment) | ||
| { | ||
| spec: { | ||
| template: { | ||
|
There was a problem hiding this comment. I found an internal reference to this variable here. There was a problem hiding this comment. This looks like perhaps it was cargo culted from here since they're using it to simply trigger a restart in this case (and they don't particularly care about reading its value). I'm tempted to change the implementation in the buddy to mimic this one (changing annotations on the template metadata). IMO having these processes/controllers mounting variables directly into containers doesn't strike me as the wisest idea. |
||
| metadata: { | ||
| annotations: { | ||
| RESTART_TRIGGER_ANNOTATION => Time.now.utc.to_datetime.rfc3339, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any specific benefit to reusing the same annotation name as kubectl or is this done just to reduce annotation clutter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fairly ambivalent about which one to use; this one cuts down noise, but can also be construed as somewhat misleading. I'm tempted to leave it as-is and change it if we get some complaints