Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

reduce k8s upgrade merge conflicts: RFC 141: Dhall investigation #10936

Closed
ggilmore opened this issue May 22, 2020 · 3 comments
Closed

reduce k8s upgrade merge conflicts: RFC 141: Dhall investigation #10936

ggilmore opened this issue May 22, 2020 · 3 comments
Assignees
Labels
estimate/3d planned/3.17 planned/3.18 RFC-141 spike Time boxed investigation meant to facilitate more granular planning.

Comments

@ggilmore
Copy link
Contributor

Background (Bash implementation)

The first Bash-based implementation of RFC 141: Reduce dev-hours needed to upgrade by moving config edits to separate directory is currently running on k8s.sgdev.org. @slimsag and I reviewed the implementation, and our full notes on the conversation are available at the bottom of the RFC.

Some selected highlights from our conversation:

Bash implementation pros:

  • The declarative specification style in params.sh makes it clear what the current modifications that are built on top of the base deploy-sourcegraph deployment
  • The custom diff script allows you to diff the output of the current transformation pipeline against your pre-existing deploy-sourcegraph YAML (useful for adoption).

Bash implementation cons:

  1. The bash-based implementation doesn't have any type system, unlike a real programming language. This makes too easy to make typo-esque mistakes that result in errors that are silent or otherwise difficult to debug. This affects both Sourcegraph developers and any users that need to write custom transformations.
  2. The way that the transformation pipeline parameters are specified is brittle (essentially passing strings around with no named parameters or type validation). It would be extremely difficult to migrate customers to use a newer version of our transformation pipeline if, for example, we needed to any of the following:
    • remove a transformation function
    • add / remove required parameters that are passed to a transformation function
    • change the order of parameters passed to a transformation function
  3. This change introduces a lot of new bash scripts to deploy-sourcegraph. Many users don't look at our documentation and read every change that we introduce to the repository (even CI / internal development changes). The new scripts will introduce a lot of moving pieces that have the potential to confuse our users to whether or not they need to pay attention to the changes (a change-log isn't sufficient here).

Next milestone

The next spike will involve exploring different configuration tools to implement a subset of deploy-sourcegraph's services and transformations to see how well they address the above concerns. (We're only implementing a subset of services to cut down on the implementation time.)

This issue is specfically tracking the Dhall - based implementation.

Services

  • sourcegraph-frontend
  • postgres
  • gitserver
  • ingress-nginx

Transformations

  • set replica counts
  • set cpu/mem/disk resource requests and limits
  • (add/set/remove) metadata annotations
  • (add/set/remove) environment variables
  • set namespaces
  • edit the docker hub base URL
  • set image tags across all services at once
  • assign a static ip address to the ingress-nginx service

Additionally, the implementation should:

  • reduce the amount of code/files in the deploy-sourcegraph repository that customers must maintain and look at
  • offer a single command that generates YAML "like" what's currently in deploy-sourcegraph's "base" folder today
    • The output can't just be a single file with every resource in a list
    • The YAML file names must be "human-friendly"

There is still an open question how about how we're supposed to dogfood this implementation. My first idea is to have someone else on @sourcegraph/distribution take my initial implementation and implement the remaining services and transformation that k8s.sgdev.org requires.

@ggilmore ggilmore self-assigned this May 22, 2020
@ggilmore ggilmore added this to the 3.17 milestone May 22, 2020
@ggilmore ggilmore added okr/distribution/admin-experience spike Time boxed investigation meant to facilitate more granular planning. labels May 22, 2020
@ggilmore ggilmore changed the title RFC 141 Implementation Round 2: Dhall spike: RFC 141 Implementation Round 2: Dhall May 22, 2020
@slimsag slimsag changed the title spike: RFC 141 Implementation Round 2: Dhall RFC 141: spike: Implementation Round 2: Dhall Jun 3, 2020
@uwedeportivo
Copy link
Contributor

Dear all,

This is your release captain speaking. 🚂🚂🚂

Branch cut for the 3.17 release is scheduled for tomorrow.

Is this issue / PR going to make it in time? Please change the milestone accordingly.
When in doubt, reach out!

Thank you

@slimsag slimsag modified the milestones: 3.17, 3.18 Jun 22, 2020
@slimsag slimsag changed the title RFC 141: spike: Implementation Round 2: Dhall RFC 141: reduce k8s upgrade merge conflicts: Dhall investigation Jun 23, 2020
@slimsag slimsag changed the title RFC 141: reduce k8s upgrade merge conflicts: Dhall investigation reduce k8s upgrade merge conflicts: RFC 141: Dhall investigation Jun 23, 2020
@daxmc99
Copy link
Contributor

daxmc99 commented Jul 13, 2020

Dear all,

This is your release captain speaking. 🚂🚂🚂

Branch cut for the 3.18 release is scheduled for tomorrow.

Is this issue / PR going to make it in time? Please change the milestone accordingly.
When in doubt, reach out!

Thank you

@pecigonzalo pecigonzalo removed this from the 3.18 milestone Aug 14, 2020
@pecigonzalo
Copy link
Contributor

Investigation is finished, we have decided to implement Dhall and we will be tracking that in https://github.com/orgs/sourcegraph/projects/71

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
estimate/3d planned/3.17 planned/3.18 RFC-141 spike Time boxed investigation meant to facilitate more granular planning.
Projects
None yet
Development

No branches or pull requests

5 participants