Skip to content

Conversation

@loujar
Copy link
Contributor

@loujar loujar commented Feb 28, 2023

This change is proposed because:

  1. Symbols and Searcher were switched from Deployment Controllers to StatefulSets in PR 242
  2. Kubernetes introduced StatefulSet support for the minReadySeconds configuration in release v1.22
  3. Sourcegraph docs advertise support for Kubernetes 1.19+, so we shouldn't ship default k8s configurations that are unsupported in 1.19

Alternative solutions could involve making minReadySeconds a configurable, optional helm value for these STSs, or possibly reverting these two services back to Deployments if we absolutely want to enforce this setting. The impact of removing this setting should be minimal, and we already ship other StatefulSets that do not define minReadySeconds setting.

Checklist

Test plan

  1. helm lint charts/sourcegraph/.
  2. helm template charts/sourcegraph/ | grep -i "minReadySeconds" -B 2
  3. Deployed in GKE reference sandbox to confirm

Copy link
Contributor

@scjohns scjohns left a comment

Choose a reason for hiding this comment

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

lgtm -- just add a test plan so all the checks pass

@loujar loujar merged commit bed48f0 into main Mar 8, 2023
@loujar loujar deleted the lsj/sts-minReadySeconds branch March 8, 2023 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants