search

devshawn / kafka-gitops

🚀Manage Apache Kafka topics and generate ACLs through a desired state file.
https://devshawn.github.io/kafka-gitops
Apache License 2.0
320 stars 71 forks source link

Executing Apply when there are no changes should exit success, not throw PlanIsUpToDateException #28

Closed infogulch closed 4 years ago

infogulch commented 4 years ago

I created a new release to apply to a new environment after a previous release successfully applied on the dev environment. But the deploy/apply to dev (which had no changes) failed with:

2020-08-13T22:37:28.9431787Z Executing apply...
2020-08-13T22:37:28.9436938Z 
2020-08-13T22:37:30.2278932Z com.devshawn.kafka.gitops.exception.PlanIsUpToDateException: The current desired state file matches the actual state of the cluster.
2020-08-13T22:37:30.2285551Z    at com.devshawn.kafka.gitops.manager.PlanManager.validatePlanHasChanges(PlanManager.java:179)
2020-08-13T22:37:30.2287816Z    at com.devshawn.kafka.gitops.StateManager.apply(StateManager.java:93)
2020-08-13T22:37:30.2291777Z    at com.devshawn.kafka.gitops.cli.ApplyCommand.call(ApplyCommand.java:35)
2020-08-13T22:37:30.2292939Z    at com.devshawn.kafka.gitops.cli.ApplyCommand.call(ApplyCommand.java:19)
2020-08-13T22:37:30.2294005Z    at picocli.CommandLine.executeUserObject(CommandLine.java:1783)
2020-08-13T22:37:30.2295589Z    at picocli.CommandLine.access$900(CommandLine.java:145)
2020-08-13T22:37:30.2296457Z    at picocli.CommandLine$RunLast.handle(CommandLine.java:2141)
2020-08-13T22:37:30.2297266Z    at picocli.CommandLine$RunLast.handle(CommandLine.java:2108)
2020-08-13T22:37:30.2298094Z    at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:1975)
2020-08-13T22:37:30.2298922Z    at picocli.CommandLine.execute(CommandLine.java:1904)
2020-08-13T22:37:30.2299804Z    at com.devshawn.kafka.gitops.MainCommand.main(MainCommand.java:69)
2020-08-13T22:37:30.6357738Z 
2020-08-13T22:37:30.6415279Z ##[error]Bash exited with code '1'.

Given that this tool is intended to be used with automation to apply desired state configuration, if no changes are required to make the current state match the desired state that should be represented as a success to the controlling automation framework.

I can imagine why one might want to fail if there were no changes and you expected there to be changes, but imo that should be an optional flag, not the default.

devshawn commented 4 years ago

Good catch! I'd probably consider this a bug, as that should be the intended behavior.

I think I'll fix this as well as output a plan file even when the desired state matches the actual state. I found that when testing this locally, that if my old plan file sticks around, you can end up double-applying if you try to apply after doing a plan with no changes. I agree that it should be an optional flag. 🚀

infogulch commented 4 years ago

Also noticed, if planning does not emit any changes, it will not write anything to or create the indicated plan file. For automation I think it would be better to emit an empty plan file (or a plan with an empty list etc).

devshawn commented 4 years ago

Yes, I also noticed that yesterday as I was addressing this issue. Both issues have been fixed and released in 0.2.7. 🚀

Thanks for the report 😄