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

Improve the platform detection for HPA #1597

Closed
iblancasa opened this issue Mar 21, 2023 · 0 comments · Fixed by #1668
Closed

Improve the platform detection for HPA #1597

iblancasa opened this issue Mar 21, 2023 · 0 comments · Fixed by #1668
Labels
area:collector Issues for deploying collector help wanted Extra attention is needed

Comments

@iblancasa
Copy link
Contributor

iblancasa commented Mar 21, 2023

While checking the problem described in #1418, we found the autodetection for HPA is not 100% correct.

The configuration is set as a value in the autoDetect struct (internal/config/main.go). So, if we copy an instance of the Config struct (like passing it by value in a method) and update the copy (from a different Goroutine for instance), the HPA value will not be reflected in the copy. This problem was solved previously for other components from Autodetect (#1206 (comment)).

To solve this problem, we can follow the same approach done for OpenShift Routes in #1463.

So, for instance, in the (c *Config) AutoDetect method, instead of:

	hpaVersion, err := c.autoDetect.HPAVersion()
	if err != nil {
		return err
	}
	c.autoscalingVersion = hpaVersion

We should do the same as we do for the OpenShift Routes:

	hpaV, err := c.autoDetect.HPAVersion()
	if err != nil {
		return err
	}
	if c.hpaVersion.Get() != hpaV {
		c.logger.V(1).Info("HPA version detected", "version", hpaV)
		c.hpaVersion.Set(hpaV)
		if err = c.onHPAVersion.Do(); err != nil {
			// Don't fail if the callback failed, as auto-detection itself worked.
			c.logger.Error(err, "configuration change notification failed for callback")
		}
	}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:collector Issues for deploying collector help wanted Extra attention is needed
Projects
None yet
2 participants