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.

Sign up for GitHub

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

Jump to bottom

including processes using excluded server list #1857

Merged
merged 11 commits into from
Nov 23, 2023
Merged

including processes using excluded server list #1857

merged 11 commits into from
Nov 23, 2023

Conversation

09harsh
Copy link
Contributor

@09harsh 09harsh commented Oct 20, 2023 •
edited by johscheuer
Loading

Description

Updated the existing code to include processes using excluded server list.

Fixes: #1854

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Discussion

Are there any design details that you would like to discuss further?
N/A

Testing

Added unit tests

Documentation

N/A

Follow-up

N/A

@09harsh 09harsh requested a review from johscheuer October 20, 2023 11:26
johscheuer
johscheuer reviewed Oct 20, 2023
View reviewed changes
controllers/remove_process_groups.go Outdated Show resolved Hide resolved
controllers/remove_process_groups.go Outdated Show resolved Hide resolved
controllers/remove_process_groups.go Outdated Show resolved Hide resolved
controllers/remove_process_groups.go Outdated Show resolved Hide resolved
pkg/fdbadminclient/mock/admin_client_mock.go Outdated Show resolved Hide resolved
@09harsh 09harsh requested a review from johscheuer October 20, 2023 13:54
@johscheuer johscheuer added the bug Something isn't working label Oct 20, 2023
@foundationdb-ci
Copy link

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

  • Commit ID: 7f630c6
  • Duration 4:08:15
  • 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: 1ccc041
  • Duration 2:49:53
  • 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: 680a667
  • Duration 4:08:09
  • 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: 602ef8b
  • Duration 2:39:41
  • 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)

johscheuer
johscheuer reviewed Oct 23, 2023
View reviewed changes
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.

Do you think there is a way to add some additional checks in one of the e2e tests for this behaviour?

controllers/remove_process_groups.go Outdated Show resolved Hide resolved
controllers/remove_process_groups.go Outdated Show resolved Hide resolved
controllers/remove_process_groups_test.go Show resolved Hide resolved
controllers/remove_process_groups_test.go Show resolved Hide resolved
controllers/remove_process_groups_test.go Outdated Show resolved Hide resolved
controllers/remove_process_groups_test.go Outdated Show resolved Hide resolved
controllers/remove_process_groups_test.go Outdated Show resolved Hide resolved
controllers/remove_process_groups_test.go Outdated Show resolved Hide resolved
controllers/remove_process_groups_test.go Outdated Show resolved Hide resolved
pkg/fdbadminclient/mock/admin_client_mock.go Outdated Show resolved Hide resolved
@foundationdb-ci
Copy link

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

  • Commit ID: a23dddd
  • Duration 2:45:25
  • 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: 072c9be
  • Duration 2:51:14
  • 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)

@foundationdb-ci
Copy link

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

  • Commit ID: ffe387b
  • Duration 2:41:40
  • 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 requested changes Nov 6, 2023
View reviewed changes
controllers/remove_process_groups.go Outdated Show resolved Hide resolved
pkg/fdbadminclient/admin_client.go Outdated Show resolved Hide resolved
@09harsh 09harsh requested a review from johscheuer November 7, 2023 05:47
johscheuer
johscheuer reviewed Nov 7, 2023
View reviewed changes
controllers/remove_process_groups.go Outdated Show resolved Hide resolved
pkg/fdbadminclient/admin_client.go Outdated Show resolved Hide resolved
@foundationdb-ci
Copy link

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

  • Commit ID: c58056f
  • Duration 2:32:16
  • 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)

@09harsh 09harsh requested a review from johscheuer November 21, 2023 08:52
johscheuer
johscheuer reviewed Nov 21, 2023
View reviewed changes
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.

The code changes LGTM. Can you make sure to rebase on the current main branch to pull in all new changes and e2e tests.

Do you thing we should add any new/additional e2e tests for this change?

@foundationdb-ci
Copy link

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

  • Commit ID: 8d09040
  • Duration 2:47:44
  • 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)

@foundationdb-ci
Copy link

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

  • Commit ID: 85ea897
  • Duration 2:39:17
  • 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)

@09harsh 09harsh force-pushed the include_use_excluded_server branch from 85ea897 to 95091f1 Compare November 22, 2023 04:37
@foundationdb-ci
Copy link

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

  • Commit ID: 95091f1
  • Duration 2:01:31
  • 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: 16af042
  • Duration 1:45:24
  • 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)

@09harsh 09harsh requested a review from johscheuer November 23, 2023 07:22
johscheuer
johscheuer approved these changes Nov 23, 2023
View reviewed changes
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 👍

@johscheuer johscheuer merged commit c73f676 into FoundationDB:main Nov 23, 2023
8 checks passed
@09harsh 09harsh deleted the include_use_excluded_server branch November 23, 2023 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@johscheuer johscheuer johscheuer approved these changes

@JoshuaMcManus JoshuaMcManus Awaiting requested review from JoshuaMcManus

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Include processes should make use of Excluded server list instead of iterating over all the ProcessGroups
3 participants
@09harsh @foundationdb-ci @johscheuer