Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Evaluate applied-checksum after setting needApplied flag #126

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

anmazzotti
Copy link

We are running into some issues when the system-agent tries to re-apply remote plans that have been already applied.

This happens whenever the system-agent is restarted or the entire host is rebooted.

Some logic is already there to suppress re-execution based on secret resource version:

			if w.lastAppliedResourceVersion == secret.ResourceVersion && !wasFailedPlan {
				logrus.Debugf("[K8s] last applied resource version (%s) did not change. running probes, skipping apply.", w.lastAppliedResourceVersion)
				needsApplied = false
			}

However the w.lastAppliedResourceVersion is an in-memory value and a service restart will blank it again.
Therefore evaluating the applied-checksum is a more reliable way for remote plans.

@anmazzotti
Copy link
Author

@davidcassany FYI

@Oats87
Copy link
Collaborator

Oats87 commented Sep 21, 2023

@anmazzotti

This is actually intentional behavior. The purpose for this is to force apply a plan if the system-agent is restarted or the node is rebooted.

I would be open to making this configurable; however, as we don't consider this behavior a bug, changing its default behavior is not acceptable

@anmazzotti
Copy link
Author

Thank you @Oats87
I pushed an additional commit that introduces a flag to skip remote plans that have been already applied.

Since the flag is false by default this should not impact the current logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants