feat: global repo params #3379
feat: global repo params #3379
Conversation
pseudomorph
force-pushed
the
parallel_operation_config
branch
from
03d8b0b to
4d7f583
Compare
|
@pseudomorph thank you these changes! I noticed that the test section above is not filled out. Could you describe there how this was tested ? Also why would the global server config override the repo config? I'd imaging the repo config would override the server config, no? |
|
@nitrocode - No problem! WRT repo config not overriding global, I was mirroring the behavior of |
|
Before you make any changes, allow me to request the approvers to respond first @jamengual @GenPage . I defer to you folks for the best consistent approach. I had no idea that the global server flag would override the repo config. I thought it was the other way around. |
|
I agree with @nitrocode the repo.yaml should be able to override the server config flags, we have started to add more config options to the repo.yaml to make it easier to configure atlantis. |
|
That order of precedence make most sense to me too. Just wanted to shoot for consistency. This is what lead me to believe my previous statement: https://github.com/runatlantis/atlantis/blob/main/server/events/automerger.go#L49-L53 Shall I open an issue to correct this behavior, or include it here? |
|
@pseudomorph Please do, we definitely want to be consistent. I'll merge this if there's no other concerns @nitrocode @jamengual |
|
👍 I'll update this PR before EOD. |
|
Actually, It may take some time, as we need to consider the ternary condition of true, false, or nil at the repoconfig level. |
|
Hmm. Seems to me that struct: https://github.com/runatlantis/atlantis/blob/main/server/core/config/valid/global_cfg.go#L95 It is functional in the command parser. Would it make sense to remove this, since all configuration possibilities should be covered through the hierarchical usercfg and repoconfig |
|
Actually, this param doesn't seem to be connected to anything in user config. |
|
Re-reviewing this today |
|
@pseudomorph please
|
|
I'll try to get this sorted today. |
| // only automerge if all projects have automerge set; or if global automerge is set and there are no projects. | ||
| automerge := c.GlobalAutomerge | ||
| if len(projectCmds) > 0 { | ||
| for _, prjCmd := range projectCmds { |
There was a problem hiding this comment.
Actually, I suppose the previous condition still holds. All projects would be contained in the same repo, so picking the first one should be sufficient.
|
@GenPage @nitrocode @jamengual - Any thoughts here? This has been running for some time in our production environment. |
|
Thanks! |
what
why
Would like to be able to configure this parameter server-side to allow fast rollback and not rely on convergence.
tests
Tests added to cover various combinations of setting the three parameters (automerge, parallel_plan, parallel_apply):
The ability to configure parallel plans in usercfg has been used in a production environment for a couple weeks.
references