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

RFC 141: spike: Implementation Round 2: Cue #10937

Closed
ggilmore opened this issue May 22, 2020 · 1 comment
Closed

RFC 141: spike: Implementation Round 2: Cue #10937

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

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 Cue - 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 added team/distribution estimate/3d spike Time boxed investigation meant to facilitate more granular planning. planned/3.17 labels May 22, 2020
@ggilmore ggilmore added this to the 3.17 milestone May 22, 2020
@ggilmore ggilmore self-assigned this May 22, 2020
@ggilmore ggilmore changed the title RFC 141 Implementation Round 2: Cue spike: RFC 141 Implementation Round 2: Cue May 22, 2020
@slimsag slimsag changed the title spike: RFC 141 Implementation Round 2: Cue RFC 141: spike: Implementation Round 2: Cue Jun 3, 2020
@ggilmore
Copy link
Contributor Author

ggilmore commented Jun 8, 2020

We're going to move on from Cue as a solution for RFC 141. Cue is very exciting, and I'd all for using it on our internal infrastrucutre if we didn't have to also consider our external users. I really wish we had this problem a year from now since the warts I've seen entirely stem from cue's young age:

  • No IDE support
  • No true diff support (critical for migration and adoption)
  • No equivalent of go modules-esque package management (This is tracked at Package distribution cuelang/cue#409 , but one of our requirements is limiting the amount of files that our users have to manage)
  • Since cue is so young, it still introduces breaking changes like switching the definition syntax. Sometimes, they include helper utilities like cue fix to ease migrations, but we can't assume that they will/can do so for every major change or that they'll cover every case.
  • Cue's documentation is both dense and sometimes incomplete. There were some operators that didn't seem to have any explicit documentation, and I had to resort looking at code samples and inferring their function.

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

No branches or pull requests

2 participants