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

Ensure we call exclude only on missing processes #1550

Merged

Conversation

johscheuer
Copy link
Member

Description

We've seen some issues with the frequent calling of the fdbcli exclude command to check if a process is excluded from the operator side. We already have logic in the operator code to check if a process is safe to delete (serves no roles) based on the cluster status. The only case where we want to call the exclude command is when the operator has an address that is missing in the cluster status json.

Type of change

Please select one of the options below.

  • New feature (non-breaking change which adds functionality)

Discussion

Most of the code changes are unit tests to ensure we cover all important cases.

Testing

Unit tests and the CI pipeline will run the e2e tests.

Documentation

We don't have a dedicated doc for how the operator handles exclusions, I think it would be a good idea to start a doc in a different PR: #1549.

Follow-up

@johscheuer johscheuer added bug Something isn't working technical-debt labels Mar 24, 2023
@foundationdb-ci
Copy link

Result of fdb-kubernetes-operator-pr on Linux CentOS 7

  • Commit ID: 63f12d2
  • Duration 1:48:02
  • Result: ✅ SUCCEEDED
  • Error: N/A
  • Build Log terminal output (available for 30 days)
  • Build Workspace zip file of the working directory (available for 30 days)

@johscheuer johscheuer merged commit 5d13b5b into FoundationDB:main Mar 24, 2023
@johscheuer johscheuer deleted the ensure-we-call-exclude-only-missing branch March 24, 2023 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working technical-debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants