Decision: allow singleton test fixtures #559
Conversation
github-actions
bot
added
component: demo services
grayside
added
do not merge
|
This seems like a proposal to codify our implicit decisions, but I find the cattle/pet terminology doesn't match my perspective so I think we need more discussion before this can move forward. The Cattle/Pet metaphor as I understand it is about automation & ephemerality vs. hand-managed and urgently protected. I see our reuse of projects as primarily a concession to security policies, user convenience, and limitations of terraform, but I don't think of them as pets. I consider it a team maintenance bug (as opposed to an Emblem project bug) that we don't have something in place to guide the project creation process more clearly, and provide some kind of consistent configuration oversight. |
|
TODO we need to document:
|
| @@ -0,0 +1,73 @@ | |||
| # Allow "pet" test fixtures | |||
There was a problem hiding this comment.
Because this is not just a point in time decision, but an ongoing guideline for related decisions in the future, it would be good to add a reference to this decision record as a design philosophy: https://github.com/GoogleCloudPlatform/emblem/blob/main/CONTRIBUTING.md#design--project-philosophy
| ### Emblem is owned by a single team | ||
| While Emblem is designed with a service-oriented architecture, all of Emblem's components are (currently) owned by the same team. | ||
|
|
||
| > Consequently, optimizing our test fixtures for multi-team collaboration isn't as important as minimizing test development/maintenance costs. |
There was a problem hiding this comment.
I'm not sure I follow this. Is the idea that in a multi-team scenario, additional automation of test fixtures is more reasonable to avoid duplicating work? If so I think this needs to be more explicit.
There was a problem hiding this comment.
I was thinking moreso large numbers of parallel test runs; I added that here to be explicit.
|
|
||
| We'll use _programmatically provisioned_ fixture resources wherever practical. These resources can be made ephemeral ("cattle") if necessary. | ||
|
|
||
| In some situations, _manual provisioning_ of centralized ("pet") resources will be simpler to implement (if not outright required). Usually, these resources **cannot** be made ephemeral and will need to be reused between test runs. |
There was a problem hiding this comment.
Could this be a bit more specific so it's more of an actionable guideline in the future? Which Cloud resources can be reusable instead of ephemeral?
There was a problem hiding this comment.
Added some examples, PTAL.
|
|
||
| ### Revisiting this Decision | ||
|
|
||
| If the organizational structure behind Emblem changes (for example, Emblem comes under the collective ownership of multiple different teams), we will re-evaluate this decision. |
There was a problem hiding this comment.
Organizational change is just one of several reasons to revisit. Really it's about maintenance overhead. If we find the maintenance overhead of permanent resources grows higher than the cost of automation, we'd revisit. The cost of automation might also go down over time, such as through the introduction of terraform state to make teardown operations simpler (#97)
There was a problem hiding this comment.
I agree in principle, but I find it hard to imagine such a maintenance cost > cost of automation scenario that doesn't involve a change in org structure.
Nevertheless, I reworded this section - so PTAL. 👍
github-actions
bot
added
the
component: experimental
github-actions
bot
removed
the
component: experimental
ace-n
changed the title
There was a problem hiding this comment.
Thank you for making changes to this proposed decision record. I think you've understood what we talked about outside the PR.
My primary remaining feedback is more subtle: we're not using fixed test resources as a design philosophy, parallelized tests are not outside scope. We've made a decision to accept fixed resources of specific kinds and not pursue the engineering for something more robust because we don't have resources to do so without postponing higher priorities.
| ## Constraints | ||
|
|
||
| ### Some resources MUST be singletons | ||
| While most of Emblem's Google Cloud resources are provisioned programmatically using Terraform, some (such as the Google Cloud projects themselves) are currently intended to be provisioned manually. |
There was a problem hiding this comment.
non-blocking: This could be improved by explaining "why". I believe the answer is a blend:
- Because these things were prototyped in this way and the engineering investment to generalize from the prototype wasn't made
- Because organization policy constraints make spinning up ad hoc projects challenging.
There was a problem hiding this comment.
I would argue it's more the latter - the former is pretty easy to do IMO, while the latter is largely (for now) out of our control.
(In fact, I had done it that way in an earlier prototype! 🙂 )
|
|
||
| ### Revisiting this Decision | ||
|
|
||
| Currently, the ongoing cost of maintaining long-lived resources is very low. Even though "pluralizing" these resources is likely a _one-time_ cost, the core Emblem team doesn't believe that cost is justified. |
There was a problem hiding this comment.
Another instance of the discussion above, it's about relative prioritization and limited time.
ace-n
added
the
automerge: exact
gcf-merge-on-green
bot
dismissed
grayside’s stale review
This review does not reference the most recent commit, and you are using the secure version of merge-on-green. Please re-review the most recent commit.
grayside
added
automerge
|
Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot. |
|
Merge-on-green is not authorized to push to this branch. Visit https://help.github.com/en/github/administering-a-repository/enabling-branch-restrictions to give gcf-merge-on-green permission to push to this branch. |
This codifies the trend Emblem has been following + previous conversations around this topic.
Googlers: this fixes b/233379931