feat: log resource drift warnings in all workspace builds #18355
Conversation
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
There was a problem hiding this comment.
We have to be careful about sending logs with elevated level. Provisionerd logs them directly at the level we get from the provisioner, which then will end up in operators log streams. That's especially problematic if provisionerd is embedded in Coderd.
I get that, arguably, if there is drift while you are claiming a prebuild then you are doing something wrong. But that isn't true of builds generally, and it's a bad idea for us to emit a bunch of WARN logs to core Coder operators based on stuff happening down in templates and builds, as template authorship might be delegated out to teams that are clients of the Coder platform.
The quickest way forward is to just not emit at WARN for this kind of stuff.
A more flexible and general solution would be to introduce some sort of log scope, to distinguish warnings and errors with builds and templates from warnings and errors with the provisioner. Only the latter should be emitted at WARN/ERROR level into the provisioner's log stream. The former can be emitted into the build logs at the given level so we display them appropriately in the UI.
|
@spikecurtis thanks for your thoughts; I'll get back to this hopefully tomorrow or early next week. |
|
OK, I've got time again to look into this. @spikecurtis and I discussed, and we're going to go ahead with:
The rationale for this is that drift is a Big Deal™️ for prebuilds, but maybe not in other cases. We may have to revisit this later to handle non-prebuilds cases in a more holistic way (like with the scope idea Spike presented), but for now this is good enough. |
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
|
Pushed up my WIP, but we need to make a call on #16999 (comment) before proceeding. |
Closes #16999
#17571 added the ability to detect and log Terraform state drift in workspace builds. We decided to only display these logs for prebuilds, initially, since prebuilds are most likely to be negatively impacted by state drift.
All output from Terraform is shown, and lines including
# forces replacementwill be marked asWARN.This PR removes the above condition and improves test coverage.
We might consider adding a hidden flag to disable this logging, in case some operators find this objectionable?
Disclaimer: credit to Claude Code for initial draft of the tests.