Skip to content

chore: more moderate probes in hmeta#55

Merged
g1eny0ung merged 4 commits intohstreamdb:mainfrom
g1eny0ung:update-hmeta-probes
Oct 13, 2023
Merged

chore: more moderate probes in hmeta#55
g1eny0ung merged 4 commits intohstreamdb:mainfrom
g1eny0ung:update-hmeta-probes

Conversation

@g1eny0ung
Copy link
Contributor

@g1eny0ung g1eny0ung commented Oct 12, 2023

Similar to #52. Refer to https://github.com/rqlite/kubernetes-configuration/blob/master/statefulset-3-node.yaml and test with a 3 hmeta cluster, increase the InitialDelaySeconds to reduce unnecessary checks.

@g1eny0ung g1eny0ung requested a review from Rory-Z as a code owner October 12, 2023 03:55
@codecov
Copy link

codecov bot commented Oct 12, 2023 •
edited
Loading

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (e885e2a) 84.17% compared to head (ce04162) 84.44%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main      #55      +/-   ##
==========================================
+ Coverage   84.17%   84.44%   +0.26%     
==========================================
  Files          31       31              
  Lines        2054     2057       +3     
==========================================
+ Hits         1729     1737       +8     
+ Misses        231      227       -4     
+ Partials       94       93       -1
Files Coverage Δ
controllers/add_hmeta.go 90.90% <100.00%> (+0.21%) ⬆️
controllers/update_hmeta_status.go 77.50% <ø> (ø)
controllers/utils.go 91.66% <100.00%> (ø)
internal/admin/client.go 62.50% <100.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@g1eny0ung
Copy link
Contributor Author

g1eny0ung commented Oct 12, 2023

I checked failed tests and it seems they failed because of a timeout bootstrap.

@g1eny0ung
Copy link
Contributor Author

g1eny0ung commented Oct 12, 2023

I checked failed tests and it seems they failed because of a timeout bootstrap.

The reason was that I changed the port name of the hmeta service, then I found we hardcoded the name to port. To solve this problem, I extracted a constants package to place the default container port of hmeta for other package usages.

Will add tests later.

@g1eny0ung
Copy link
Contributor Author

g1eny0ung commented Oct 13, 2023

I found we haven't added tests in the internal/admin package, supplementing them would be a lot of work, so I opened a new PR to handle it (#56).

@Rory-Z Shall we merge this PR first due to changes that didn't break test coverage much?

@Rory-Z
Copy link
Collaborator

Rory-Z commented Oct 13, 2023

@g1eny0ung I think we can merge this PR, could you please Squash and merge this

@g1eny0ung
Copy link
Contributor Author

g1eny0ung commented Oct 13, 2023

@g1eny0ung I think we can merge this PR, could you please Squash and merge this

I don't have the permission to merge. 🤣 Can I be granted one?

@g1eny0ung g1eny0ung merged commit b1a90e0 into hstreamdb:main Oct 13, 2023
@g1eny0ung g1eny0ung deleted the update-hmeta-probes branch October 13, 2023 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Rory-Z Rory-Z Rory-Z 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.

2 participants

@g1eny0ung @Rory-Z