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

fix(gcp): Relaxed health check for GCP accounts #6200

Merged
merged 1 commit into from
May 1, 2024

Conversation

christosarvanitis
Copy link
Member

The majority of cloud providers (AWS, K8s) and dockerRegistry have already implementations for relaxed health checks via a configuration property in the respective provider block.

On GCP provider though it was attempted to be addressed by #6093 which however was not the correct implementation since the Health check is registered in initialisation and it is a scheduled task.

This PR fixes the issue spinnaker/spinnaker#3923 for the GCP provider

@ovidiupopa07 ovidiupopa07 added the ready to merge Approved and ready for a merge label May 1, 2024
@mergify mergify bot added the auto merged Merged automatically by a bot label May 1, 2024
@mergify mergify bot merged commit 28599eb into spinnaker:master May 1, 2024
23 checks passed
@jasonmcintosh
Copy link
Member

Ideally this gets changed a bit in the future. With the Bean, it would disable the thread & scheduled job which would output a log message every 30 seconds. Since it was also a component that Bean disable didn't help this. Doing this method, we still get the log message, the scheduled run that outputs the log message every 30 seconds, plus the whole if check thing. It's MINOR but... we shouldn't really load these health checks this way. Be good to fix all of them to disable them vs. the current implementation.

@jasonmcintosh
Copy link
Member

@Mergifyio backport release-1.28.x

Copy link
Contributor

mergify bot commented May 1, 2024

backport release-1.28.x

✅ Backports have been created

@jasonmcintosh
Copy link
Member

@Mergifyio backport release-1.34.x release-1.33.x release-1.32.x

Copy link
Contributor

mergify bot commented May 1, 2024

backport release-1.34.x release-1.33.x release-1.32.x

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request May 1, 2024
(cherry picked from commit 28599eb)

# Conflicts:
#	clouddriver-google/src/main/groovy/com/netflix/spinnaker/clouddriver/google/health/GoogleHealthIndicator.groovy
mergify bot pushed a commit that referenced this pull request May 1, 2024
mergify bot pushed a commit that referenced this pull request May 1, 2024
mergify bot pushed a commit that referenced this pull request May 1, 2024
mergify bot added a commit that referenced this pull request May 7, 2024
(cherry picked from commit 28599eb)

Co-authored-by: Christos Arvanitis <christos.arvanitis@armory.io>
mergify bot added a commit that referenced this pull request May 7, 2024
(cherry picked from commit 28599eb)

Co-authored-by: Christos Arvanitis <christos.arvanitis@armory.io>
mergify bot added a commit that referenced this pull request May 7, 2024
(cherry picked from commit 28599eb)

Co-authored-by: Christos Arvanitis <christos.arvanitis@armory.io>
mergify bot added a commit that referenced this pull request May 7, 2024
* fix(gcp): Relaxed health check for GCP accounts (#6200)

(cherry picked from commit 28599eb)

# Conflicts:
#	clouddriver-google/src/main/groovy/com/netflix/spinnaker/clouddriver/google/health/GoogleHealthIndicator.groovy

* fix: Fixes PR to match 1.28 versions of credential repository APIs

---------

Co-authored-by: Christos Arvanitis <christos.arvanitis@armory.io>
Co-authored-by: Jason McIntosh <jason.mcintosh@harness.io>
aman-agrawal pushed a commit to aman-agrawal/clouddriver that referenced this pull request Jul 5, 2024
…nnaker#6203)

(cherry picked from commit 28599eb)

Co-authored-by: Christos Arvanitis <christos.arvanitis@armory.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto merged Merged automatically by a bot ready to merge Approved and ready for a merge target-release/1.35
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants