Reloader icon indicating copy to clipboard operation
Reloader copied to clipboard

Adding a annotation flag to trigger rollout on change of cm/secrets

Open wololowarrior opened this issue 1 year ago • 1 comments

#724 This pr removed the functionality of triggering a rollout strategy on change of configmap/secrets. But there are some cases when we want to trigger a rollout instead of just restarting the pod.

As part of this pr,

  1. Adding a new annotation reloader.stakater.com/triggerRollout if set to true will trigger a rollout strategy for that rollout. The default can be unset or false, so current behaviour should continue on.

Example Setting as true

apiVersion: argoproj.io/v1alpha1
kind: Rollout
metadata:
  name: simple-rollout
  annotations:
    reloader.stakater.com/auto: "true"
    reloader.stakater.com/triggerRollout: "true"
spec:
  replicas: 2

wololowarrior avatar Oct 16 '24 04:10 wololowarrior

@MuneebAijaz Can i get some maintainers reviews?

wololowarrior avatar Oct 21 '24 06:10 wololowarrior

@MuneebAijaz @karl-johan-grahn Would appreciate 👀 and reviews on this whenever possible!

wololowarrior avatar Nov 02 '24 15:11 wololowarrior

@wololowarrior can you please add a unit test for this? You can take inspiration from existing tests

karl-johan-grahn avatar Nov 27 '24 09:11 karl-johan-grahn

@karl-johan-grahn Added a test for rollout. Weirdly there weren't any tests for it so more code than usual. I've added a test for one part of the branch. Which had the old behaviour of triggering a rollout strategy.

I also tried writing a test for the case where its a rolling update, but that test doesn't seem to pass. I suspect this is because the stubbed rollout client isnt built to handle restartAt cases.

wololowarrior avatar Jan 05 '25 06:01 wololowarrior

Cross-referencing https://github.com/stakater/Reloader/pull/817 that addresses the same issue https://github.com/stakater/Reloader/pull/724

karl-johan-grahn avatar Jan 08 '25 09:01 karl-johan-grahn

@karl-johan-grahn fixed ci. Which pr do you guys prefer?

wololowarrior avatar Jan 13 '25 08:01 wololowarrior

Hi @wololowarrior can you rebase your PR so we can see if there is any remaining overlap compared to the other PR that got merged first?

karl-johan-grahn avatar Feb 12 '25 09:02 karl-johan-grahn