terraform-cdk icon indicating copy to clipboard operation
terraform-cdk copied to clipboard

Add locations to watch via Constructs

Open DanielMSchmidt opened this issue 3 years ago • 8 comments

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

Currently the watch command is configured on a project basis in the cdktf.json through globs. This means if there are a variety of different stacks and my watch command is limited to a small subset, then file changes related to not-currently-watched stacks might trigger changes. To improve this we'd need to add stack-level watch declarations. For this I'd propose a Construct based API like this:

import { WatchableDirectory, WatchableFile } from "cdktf";
import {Construct} from "constructs";

export class MyReactApp extends Construct {
  constructor(scope: Construct, name: string, dir:string) {
    super(scope, name);
    new WatchableDirectory(this, "src-folder", {
      path: path.resolve(dir, "src")
    })
   new WatchableFile(this, "package.json", {
     path: path.resolve(dir, "package.json")
   })
  }
}

This will add a filesToWatch section to each stack in the manifest.json that cdktf watch can watch and adjust the watched files with. Exposing such an API via construct allows package authors to influence the watch behaviour so that we get into a "just works" state if everything is configured correctly

References

DanielMSchmidt avatar Mar 29 '22 08:03 DanielMSchmidt

Currently the watch command is configured on a project basis in the cdktf.json through globs. This means if there are a variety of different stacks and my watch command is limited to a small subset, then file changes related to not-currently-watched stacks might trigger changes.

Do you have a specific example for this? Trying to understand the use-case better. Is it driven by assets (Functions, Docker builds, ...) or really the stack itself?

skorfmann avatar Mar 30 '22 09:03 skorfmann

In my vision higher level constructs (and basic constructs like Asset) would use this new construct, I think it would rarely be used at the top-level. The advantage of using a separate construct here instead of building this functionality only directly into assets is an increased flexibility. If I read a file with fs.readFileSync during synth, I can specify it as a file to watch via the construct and a change can trigger a redeploy

DanielMSchmidt avatar Mar 30 '22 10:03 DanielMSchmidt

In my vision higher level constructs (and basic constructs like Asset) would use this new construct, I think it would rarely be used at the top-level. The advantage of using a separate construct here instead of building this functionality only directly into assets is an increased flexibility. If I read a file with fs.readFileSync during synth, I can specify it as a file to watch via the construct and a change can trigger a redeploy

If I understand this correctly, this aims mainly at:

  1. Asset like constructs
  2. Potential other input files, perhaps something like a JSON config file or something similar

Could you elaborate on the benefit of explicitly defining the "watchable" files over proper change detection for the assets itself (which we don't have right now) with a broad watch configuration? I feel like I still haven't fully understood the problem this is aiming to solve.

skorfmann avatar Mar 30 '22 12:03 skorfmann

If I understand this correctly, this aims mainly at:

  1. Asset like constructs
  2. Potential other input files, perhaps something like a JSON config file or something similar

Yes, exactly.

Could you elaborate on the benefit of explicitly defining the "watchable" files over proper change detection for the assets itself (which we don't have right now) with a broad watch configuration? I feel like I still haven't fully understood the problem this is aiming to solve.

I don't think proper change detection is not a solvable problem since there is no heuristic that can be used if one broadly watches over files. It depends too much on the users setup, that's why it should be in the users power to define this via constructs. The heuristics I thought of were:

  • all files: catches also changes not related, too eagerly triggering a deploy
  • all files in git: if you use a generated file as source for an asset (with cdktf being just a part of your build pipeline) we will skip on updates
  • all files in git but diff changes in TerraformAsset of last run vs current state of that directory: This does not keep track of files included during synth except if they are included in the broad watch configuration.

Another problem I see is if folks have workflows that trigger file changes that are not related to infrastructure in the current watch configuration. Take our e2e examples with an API and a frontend. In development I might only want to watch the backend and let my frontend locally run against the development backend. Now every change I make to the frontend causes my backend to unnecessarily redeploy. For this reason I think we need stack-dependant watch configurations and the easiest way to configure them is through a construct based API.

DanielMSchmidt avatar Mar 30 '22 13:03 DanielMSchmidt

Another problem I see is if folks have workflows that trigger file changes that are not related to infrastructure in the current watch configuration. Take our e2e examples with an API and a frontend. In development I might only want to watch the backend and let my frontend locally run against the development backend. Now every change I make to the frontend causes my backend to unnecessarily redeploy

When looking at this example wouldn't it just be a matter of excluding the assets and frontend folders from the glob?

Screenshot 2022-03-30 at 16 43 33

Besides from that, what do you see as the actual goal - reducing the time it takes to synth the config, or skipping unnecessary deployments?

skorfmann avatar Mar 30 '22 14:03 skorfmann

When looking at this example wouldn't it just be a matter of excluding the assets and frontend folders from the glob?

In this example yes, since the frontends content is not managed by CDKTF. If you take an example like this where CDKTF manages the upload (in this case in one stack, but it could totally be two stacks here) then we run into the issue describes (since we might sometimes want to watch these files and sometimes not).

Besides from that, what do you see as the actual goal - reducing the time it takes to synth the config, or skipping unnecessary deployments?

I want to skip unnecessary deployments. If deployments take a moment triggering them too eagerly results in the user having to wait for a deployment to be done, plus the time of the next deployment with the new changes instead of just the latter. This can add enough friction to decide against watch even when it would make sense workflow wise.

DanielMSchmidt avatar Mar 30 '22 15:03 DanielMSchmidt

When looking at this example wouldn't it just be a matter of excluding the assets and frontend folders from the glob?

In this example yes, since the frontends content is not managed by CDKTF. If you take an example like this where CDKTF manages the upload (in this case in one stack, but it could totally be two stacks here) then we run into the issue describes (since we might sometimes want to watch these files and sometimes not).

Without having looked into that repo in depth, my gut feeling for this example is that it follows a very traditional IaC approach - which is fine - but nothing I'd optimize for. The code structure implies, that a dev would run docker compose locally anyway and infrastructure is an afterthought. If I wanted to have "cloud driven" workflow here, I'd restructure / refactor the codebase extensively. So, perhaps that's more a topic of providing good examples / best practices / templates?

Besides from that, what do you see as the actual goal - reducing the time it takes to synth the config, or skipping unnecessary deployments?

I want to skip unnecessary deployments. If deployments take a moment triggering them too eagerly results in the user having to wait for a deployment to be done, plus the time of the next deployment with the new changes instead of just the latter. This can add enough friction to decide against watch even when it would make sense workflow wise.

Ok, I see. Baking this behaviour into cdktf lib itself feels a bit weird though - in particular since it's targeting a very narrow use-case. Judging based on my experience, some from of improved asset caching would go a long way in speeding up workflows across the board - in particular when these assets trigger long running re-deployments of resources like e.g. CloudFront.

skorfmann avatar Mar 31 '22 08:03 skorfmann

If I wanted to have "cloud driven" workflow here, I'd restructure / refactor the codebase extensively. So, perhaps that's more a topic of providing good examples / best practices / templates?

Sorry for being unclear, the repository served only as an example for how users might get into a situation where there is a file that is important to watch in the context of one stacks but irrelevant for another stack. A hybrid dev workflow where the backend is deployed continuously via watch, whereas a frontend is iterated on locally is what I wanted to describe. I think this is a quite common use-case.

Some from of improved asset caching would go a long way in speeding up workflows across the board - in particular when these assets trigger long running re-deployments of resources like e.g. CloudFront.

I agree that asset caching can solve some of the issues we are experiencing right now, but I strongly disagree that it can solve all of them, see the workflow described above.

DanielMSchmidt avatar Mar 31 '22 09:03 DanielMSchmidt

I'm going to lock this issue because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

github-actions[bot] avatar Aug 13 '23 01:08 github-actions[bot]