Hello! Thank you for filing an issue.

The maintainers will triage your issue shortly.

In the meantime, please take a look at the troubleshooting guide for bug reports.

If this is a feature request, please review our contribution guidelines.

Maybe this is happening because the listener pod gets recreated by the reconciler between the return on line 292 and the next execution? So it never matches line 293 (case kerrors.IsNotFound(err))?

Hmm no, it doesn't even get there. What I think happens is that the pod terminates because the container terminates and restartPolicy is set to never. Then the Reconcile loop sees that, deletes the pod, and recreates it the next loop. But because the config secret isn't deleted, it doesn't get recreated, so it still has the old values.

Hey @hsmade, if you want to update the token, you should probably update it for the autoscaling runner set as well. But if this secret gets updated, we should probably re-create the AutoscalingListener resource which should re-create the configuration and the listener pod, mirror secret etc.

Hi!

The autoscaler runner set has:

spec:
  githubConfigSecret: github-auth

Which is the secret that is getting updated. But the changes don't get reflected in the listener.
What I find curious is that when the listener pod terminates, the pod gets deleted by the reconciler, just to get recreated with the exact same data. Including the config secret that isn't updated.
The mirror secret is actually updated automatically, and correctly.

What I suggest is that when the listener pod gets deleted, its config secret gets deleted as well.
This actually does fix the issue I'm seeing (tested this in a running environment).

PR #4033 is what I tested on my environment, and seems to fix the issue.

Hey @hsmade,

Thank you for raising the PR, but it doesn't fix the root of the problem. The listener exit could be happening due to various reasons, and re-creating the secret on each crash would put more pressure on the API server without any need for it.
I think that the appropriate fix is to track the spec hash, including the secret hash. Including both would re-create the entire AutoscalingListener instance, which would include mirroring the secret.

Would you be interested in testing this change?

The code does seem to know when the mirror secret is out of sync. Maybe that should invalidate the config secrets as well:

	mirrorSecretDataHash := mirrorSecret.Labels["secret-data-hash"]
	secretDataHash := hash.ComputeTemplateHash(secret.Data)
	if mirrorSecretDataHash != secretDataHash {
		log.Info("Updating mirror listener secret for the listener pod", "mirrorSecretDataHash", mirrorSecretDataHash, "secretDataHash", secretDataHash)
		return r.updateSecretsForListener(ctx, secret, mirrorSecret, log)
	}

I'm confused to as to why updateSecretsForListener doesn't create the config secret, only the mirror secret.
What's the purpose of having this mirror secret actually?

Hey @hsmade,

I was playing around with the fix, and I was wondering if you can simply update the values.yaml file and re-apply the new secret with it, wouldn't that solve your problem? Instead of updating the secret directly, you update it through the values.yaml file. We do have some mechanisms to understand when the values.yaml hash is changed, and it should solve your problem.

We're using argo-cd to deploy this. The token refresh happens because I'm using an external secret operator that generates it from the GH app. And the TTL for such token is just 1 hour.
But in general, I'd say it would be good to support the primary token secret to update to work?

You were right, your approach was much better! I left few comments on the PR. 😄