MLE-22135 Port Helm change to Operator #93
Conversation
pengzhouml
requested review from
Copilot,
barkhachoithani,
sumanthravipati,
vitalykorolev and
rwinieski
There was a problem hiding this comment.
Pull Request Overview
Ports the Helm-based liveness probe to the operator, adds retry logic for configure_group in the postStart hook, removes the legacy script, and updates the default HAProxy image.
- Replace Exec-based liveness probe with a TCP socket check on port 8001
- Introduce a
retryhelper and wrapconfigure_groupcalls in postStart hook - Delete obsolete
liveness-probe.shscript and bump HAProxy default image to v3.2.1
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/k8sutil/statefulset.go | Switch liveness probe from ExecAction script to TCPSocketAction |
| pkg/k8sutil/scripts/poststart-hook.sh | Add retry function and wrap configure_group calls |
| pkg/k8sutil/scripts/liveness-probe.sh | Remove obsolete liveness probe script |
| api/v1/common_types.go | Update HAProxy default image from 3.2 to 3.2.1 |
Comments suppressed due to low confidence (2)
pkg/k8sutil/scripts/poststart-hook.sh:59
- New retry logic is introduced; consider adding unit or integration tests to verify that
configure_groupis retried as expected on failure.
retry() {
api/v1/common_types.go:85
- After updating the default HAProxy image tag, regenerate CRDs or update API documentation to reflect this new default version.
// +kubebuilder:default:="haproxytech/haproxy-alpine:3.2.1"
| echo "Command failed after $retries attempts." | ||
| return $exit_code | ||
| fi | ||
| echo "Attempt $count failed. Retrying..." |
There was a problem hiding this comment.
[nitpick] Use the existing log function instead of echo so retries are recorded in the pod log and /tmp/script.log.
| echo "Command failed after $retries attempts." | |
| return $exit_code | |
| fi | |
| echo "Attempt $count failed. Retrying..." | |
| log "Error" "Command failed after $retries attempts." | |
| return $exit_code | |
| fi | |
| log "Info" "Attempt $count failed. Retrying..." |
Copilot uses AI. Check for mistakes.
| echo "Command failed after $retries attempts." | ||
| return $exit_code | ||
| fi | ||
| echo "Attempt $count failed. Retrying..." |
There was a problem hiding this comment.
[nitpick] Replace this echo with the log helper to ensure consistent logging with timestamps and PID handling.
| echo "Attempt $count failed. Retrying..." | |
| info "Attempt $count failed. Retrying..." |
Copilot uses AI. Check for mistakes.
Makefile
Outdated
| ifeq ($(VERIFY_HUGE_PAGES), true) | ||
| @echo "=====Setting hugepages value to 1280 for hugepages-e2e test" | ||
| sudo sysctl -w vm.nr_hugepages=1280 | ||
| # @echo "=====Setting hugepages value to 1280 for hugepages-e2e test" |
There was a problem hiding this comment.
setting huge pages is required to verify the huge pages.
please comment if I am missing anything here?
There was a problem hiding this comment.
@barkhachoithani , I accidentally commit these comments for my local test. I've revert chose changes back
No description provided.