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

add podName selection functions #1944

Merged

Conversation

nicmorales9
Copy link
Contributor

@nicmorales9 nicmorales9 commented Feb 19, 2024

Description

This is just a followup to #1941 which used an unsatisfying way to get podNames from processGroupIDs.

I am not sure that this is much better, but they do seem like useful functions in the transition away from selecting by processGroupID.

It will be much easier to see the changes if you view by commit to avoid looking at the second commit, which changes test pod names to include processClass in the name, which is an expectation of built-in functions (GetPodName will index-out-of-bounds).

Type of change

  • Code Cleanup

Discussion

I wonder if there should be some explicit creation function which does not allow FDB pods to have invalid names, as I suspect that it is not only GetPodName which could have undefined behavior if expectations aren't met.

Testing

Please describe the tests that you ran to verify your changes. Unit tests?
Manual testing?

Unit tests, manual test of restart.

Do we need to perform additional testing once this is merged, or perform in a larger testing environment?

Documentation

Docstrings have been added

Follow-up

Are there any follow-up issues that we should pursue in the future?
Arguably this could be progress towards not allowing useProcessGroupID and working more with podNames when possible, but we had trouble finding that issue before. Perhaps we should raise one?

Does this introduce new defaults that we should re-evaluate in the future?
No

incompatible with some test setup and will not pass tests without editing test pod /process names
to conform with pod name format expectations
@foundationdb-ci
Copy link

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

  • Commit ID: b4a879e
  • Duration 1:48:21
  • Result: ❌ FAILED
  • Error: Error while executing command: if $fail_test; then exit 1; fi. Reason: exit status 1
  • Build Log terminal output (available for 30 days)
  • Build Workspace zip file of the working directory (available for 30 days)

@foundationdb-ci
Copy link

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

  • Commit ID: ba82c9c
  • Duration 1:43:12
  • 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)

Copy link
Member

@johscheuer johscheuer left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Thanks!

@johscheuer johscheuer merged commit 625b7d8 into FoundationDB:main Feb 20, 2024
8 checks passed
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