feat(reports): enable optional reports generator deployment #191
Conversation
andrewazores
force-pushed
the
sidecar-reports
branch
from
ce8520b
to
45846bf
Compare
|
The connection flakiness actually reminds me a lot of what I observed before: So it seems the underlying HTTP client library was not really the problem, or the solution. |
|
Opened #194 to track the connection flakiness as a separate issue. |
andrewazores
force-pushed
the
sidecar-reports
branch
from
45846bf
to
9cbf3cd
Compare
andrewazores
force-pushed
the
separate-db-storage
branch
from
fad64c0
to
0d0b633
Compare
andrewazores
force-pushed
the
sidecar-reports
branch
from
9cbf3cd
to
7bece6e
Compare
andrewazores
force-pushed
the
separate-db-storage
branch
from
0d0b633
to
fdbc6b9
Compare
andrewazores
force-pushed
the
sidecar-reports
branch
2 times, most recently
from
93ee4cc
to
c5a608b
Compare
andrewazores
force-pushed
the
sidecar-reports
branch
from
c5a608b
to
b015b53
Compare
| - name: QUARKUS_REST_CLIENT_REPORTS_URL | ||
| value: {{ printf "http://%s-reports.%s.svc:%d" $fullName $.Release.Namespace (int .Values.reports.service.httpPort) }} |
There was a problem hiding this comment.
Maybe we can standardize the hostname for component services here by either using service-name or service-name.namespace.svc?
There was a problem hiding this comment.
True, it should be unnecessary to specify the namespace and .svc. I think I did this out of desperation while trying to figure out what turned out to be the label selectors bug :-)
| spec: | ||
| replicas: {{ .Values.reports.replicas }} | ||
| strategy: | ||
| type: Recreate |
There was a problem hiding this comment.
I think the strategy can be RollingUpdate here?
Since the report sidecars do not require any PVC, there should not be any issues with pods on different nodes binding to the same PVC (as in the case of cryostat), thus requiring Recreate strategy. Or there might be other reasons?
At least on the operator side, it is RollingUpdate: https://github.com/cryostatio/cryostat-operator/blob/1bdfc3e61ab7fd4e7e7759f0007a09ad27efb057/internal/controllers/common/resource_definitions/resource_definitions.go#L236
| tolerations: | ||
| {{- toYaml . | nindent 8 }} | ||
| {{- end }} | ||
| volumes: |
charts/cryostat/values.yaml
Outdated
| cpu: 2000m | ||
| ## @param reports.resources.requests.memory Memory resource request for each Pod in the Report Generator Deployment. | ||
| memory: 512Mi |
There was a problem hiding this comment.
Should these match the specifications here?
https://github.com/cryostatio/cryostat-operator/blob/main/docs/config.md#reports-options
Co-authored-by: Thuan Vo <[email protected]> Signed-off-by: Andrew Azores <[email protected]>
Fixes #185
Currently in draft mode because there is some connection flakiness ("Connection refused") on report generation requests. It seems like some misconfiguration or maybe load balancer naming conflict.