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

sink: wait for infra-agent to be ready before starting #346

Merged
merged 9 commits into from
Feb 2, 2022
Merged

Conversation

roobre
Copy link
Contributor

@roobre roobre commented Jan 26, 2022

As the title says, after upgrading the nri-bundle on my local environment, the nrk8s-ksm pod restarted 3 times while waiting for the agent to become ready. nrk8s-kubelet restarted twice. This simple trick should make a more graceful wait without alerting the user if the agent does not come up immediately.

@roobre roobre requested a review from a team January 26, 2022 19:35
@paologallinaharbur
Copy link
Member

yes, I noticed that as well, thanks for taking care of it!

cmd/nri-kubernetes/main.go Outdated Show resolved Hide resolved
src/sink/probe.go Outdated Show resolved Hide resolved
Copy link
Contributor

@gsanchezgavier gsanchezgavier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

_ = resp.Body.Close()

if resp.StatusCode != http.StatusOK {
return fmt.Errorf("%w: %d", errProbeNotOk, resp.StatusCode)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps you can get rid of the errProbeNotOk var since is just for loggin

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I planned to do so but I'm not sure if the linter would complain in this case. Since I already had the var created from a previous iteration where this error was somewhere returned) I left it there, but we can remove it if you want!

src/integration/prober/prober.go Show resolved Hide resolved
Copy link
Contributor

@gsanchezgavier gsanchezgavier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! thanks for adding the exported Func comments

roobre and others added 6 commits January 31, 2022 13:57
fixup! cosmetic refactoring
fixup! rename to wrapper.go
Since we are now waiting using the prober, errors POSTing data to the sidecar are actually concerning
Co-authored-by: Guillermo Sanchez Gavier <gsanchez@newrelic.com>
@roobre roobre merged commit 1d4c557 into main Feb 2, 2022
@roobre roobre deleted the agent-wait branch February 2, 2022 13:27
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.

3 participants