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

CLOUDP-221980: Added search nodes implementation #1392

Merged
merged 26 commits into from
Apr 26, 2024
Merged

CLOUDP-221980: Added search nodes implementation #1392

merged 26 commits into from
Apr 26, 2024

Conversation

igor-karpukhin
Copy link
Collaborator

All Submissions:

  • Have you signed our CLA?
  • Put closes #XXXX in your comment to auto-close the issue that your PR fixes (if there is one).
  • Update docs/release-notes/release-notes.md if your changes should be included in the release notes for the next release.

@roothorp roothorp force-pushed the CLOUDP-221980-search-nodes branch 2 times, most recently from 983d59e to 11ac93f Compare March 26, 2024 14:19
@roothorp roothorp marked this pull request as ready for review March 26, 2024 14:53
@roothorp roothorp requested review from helderjs and s-urbaniak March 26, 2024 14:54
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 26, 2024 •
edited
Loading

https://app.codecov.io/github/mongodb/mongodb-atlas-kubernetes/commit/2d04c27771ff404206c61e6b179eef07958e24d0

s-urbaniak
s-urbaniak requested changes Mar 26, 2024
View reviewed changes
Copy link
Collaborator

@s-urbaniak s-urbaniak left a comment

Choose a reason for hiding this comment

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

good start, my biggest concern is about potentially missing unit testing and missing normalization of search node slices.

@roothorp roothorp force-pushed the CLOUDP-221980-search-nodes branch from 2dbc392 to 27580bd Compare March 26, 2024 16:25
@roothorp roothorp requested a review from josvazg March 26, 2024 16:31
@roothorp roothorp force-pushed the CLOUDP-221980-search-nodes branch from 27580bd to 25f285a Compare April 5, 2024 09:30
igor-karpukhin and others added 25 commits April 26, 2024 10:30
@igor-karpukhin @s-urbaniak
Removed debug output
0e1d457
@igor-karpukhin @s-urbaniak
Make linter happy
1b28398
@roothorp @s-urbaniak
add e2e tests
68902f5
@roothorp @s-urbaniak
run search tests in CI
ea1e694
@roothorp @s-urbaniak
set conditions for search nodes
86298eb
@roothorp @s-urbaniak
fix tests
8d14ec1
@roothorp @s-urbaniak
fix unit tests
ac65c72
@roothorp @s-urbaniak
fix e2e tests more
cb5bbf4
@roothorp @s-urbaniak
update manifests
85d930e
@roothorp @s-urbaniak
updates as per review
e78902b
@roothorp @s-urbaniak
bump to new atlas sdk version
3d48a99
@roothorp @s-urbaniak
add more unit tests
626d3c5
@roothorp @s-urbaniak
updates per review
f201ac2
@roothorp @s-urbaniak
add unit testing
5affcc4
@roothorp @s-urbaniak
more informative conditions
dae38fa
@roothorp @s-urbaniak
centralise error logging & clean up
421a40b
@s-urbaniak
rewrite to idiomatic controller using state handlers and transitions
30a7351
@roothorp @s-urbaniak
Add descriptions to new CRD fields
0b235a1
@s-urbaniak
terminate on changes during updating state
42b4d0c
@roothorp @s-urbaniak
further unit tests
14a75b7
@s-urbaniak @josvazg
Update pkg/controller/atlasdeployment/search_nodes.go
44bd0e8 
Co-authored-by: josvaz <[email protected]>
@s-urbaniak @helderjs
Update pkg/controller/atlasdeployment/search_nodes.go
651fd29 
Co-authored-by: Helder Santana <[email protected]>
@s-urbaniak @josvazg
Update pkg/controller/atlasdeployment/search_nodes.go
f56ad22 
Co-authored-by: josvaz <[email protected]>
@roothorp @s-urbaniak
further unit tests for new states
d0da829
@roothorp @s-urbaniak
appease linter & fix comments
2d04c27
@s-urbaniak s-urbaniak force-pushed the CLOUDP-221980-search-nodes branch from d80e408 to 2d04c27 Compare April 26, 2024 08:31
@s-urbaniak s-urbaniak merged commit bacd4c5 into main Apr 26, 2024
49 checks passed
@s-urbaniak s-urbaniak deleted the CLOUDP-221980-search-nodes branch April 26, 2024 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roothorp roothorp roothorp left review comments

@josvazg josvazg josvazg approved these changes

@s-urbaniak s-urbaniak s-urbaniak approved these changes

@helderjs helderjs helderjs approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@igor-karpukhin @roothorp @s-urbaniak @helderjs @josvazg