Skip to content

Commit

Permalink
chore(bors): merge pull request #473
Browse files Browse the repository at this point in the history
473: fix(jaeger): update and use the jaeger bundled CRD r=tiagolobocastro a=tiagolobocastro

It seems otherwise there's a race condition in which CRD gets installed depending on the helm version.
We don't actually need to use our own CRD for jaeger anymore, so let's instead if the bundled one from jaeger-operator.
Updates jaeger-operator.
Note that as of now cert-manager is a required dependency for jaeger.

<!

Co-authored-by: Tiago Castro <tiagolobocastro@gmail.com>
  • Loading branch information
mayastor-bors and tiagolobocastro committed Apr 10, 2024
2 parents 3604570 + 6bf41bf commit ccc8b9f
Show file tree
Hide file tree
Showing 10 changed files with 18 additions and 85 deletions.
1 change: 1 addition & 0 deletions Jenkinsfile
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ pipeline {
}
agent { label 'nixos-mayastor' }
steps {
sh 'printenv'
sh './scripts/nix/git-submodule-init.sh --force'
sh './scripts/release.sh --skip-publish --debug --build-bins'
}
Expand Down
2 changes: 1 addition & 1 deletion chart/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ dependencies:
repository: https://charts.bitnami.com/bitnami
version: 8.6.0
- name: jaeger-operator
version: 2.25.0
version: 2.50.1
repository: https://jaegertracing.github.io/helm-charts
condition: base.jaeger.enabled
- name: loki-stack
Expand Down
4 changes: 1 addition & 3 deletions chart/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ This removes all the Kubernetes components associated with the chart and deletes
| | crds | 0.0.0 |
| https://charts.bitnami.com/bitnami | etcd | 8.6.0 |
| https://grafana.github.io/helm-charts | loki-stack | 2.9.11 |
| https://jaegertracing.github.io/helm-charts | jaeger-operator | 2.25.0 |
| https://jaegertracing.github.io/helm-charts | jaeger-operator | 2.50.1 |
| https://nats-io.github.io/k8s/helm/charts/ | nats | 0.19.14 |
| https://openebs.github.io/dynamic-localpv-provisioner | localpv-provisioner | 4.0.0 |

Expand Down Expand Up @@ -101,14 +101,12 @@ This removes all the Kubernetes components associated with the chart and deletes
| base.&ZeroWidthSpace;cache_poll_period | Cache timeout for core agent & diskpool deployment | `"30s"` |
| base.&ZeroWidthSpace;default_req_timeout | Request timeout for rest & core agents | `"5s"` |
| base.&ZeroWidthSpace;imagePullSecrets.&ZeroWidthSpace;enabled | Enable imagePullSecrets for pulling our container images | `false` |
| base.&ZeroWidthSpace;jaeger.&ZeroWidthSpace;enabled | Enable jaeger tracing | `false` |
| base.&ZeroWidthSpace;logging.&ZeroWidthSpace;color | Enable ansi color code for Pod StdOut/StdErr | `true` |
| base.&ZeroWidthSpace;logging.&ZeroWidthSpace;format | Valid values for format are pretty, json and compact | `"pretty"` |
| base.&ZeroWidthSpace;logging.&ZeroWidthSpace;silenceLevel | Silence specific module components | `nil` |
| base.&ZeroWidthSpace;metrics.&ZeroWidthSpace;enabled | Enable the metrics exporter | `true` |
| crds.&ZeroWidthSpace;csi.&ZeroWidthSpace;volumeSnapshots.&ZeroWidthSpace;enabled | Install Volume Snapshot CRDs | `true` |
| crds.&ZeroWidthSpace;enabled | Disables the installation of all CRDs if set to false | `true` |
| crds.&ZeroWidthSpace;jaeger.&ZeroWidthSpace;enabled | Install Jaeger CRDs | `true` |
| csi.&ZeroWidthSpace;controller.&ZeroWidthSpace;logLevel | Log level for the csi controller | `"info"` |
| csi.&ZeroWidthSpace;controller.&ZeroWidthSpace;preventVolumeModeConversion | Prevent modifying the volume mode when creating a PVC from an existing VolumeSnapshot | `true` |
| csi.&ZeroWidthSpace;controller.&ZeroWidthSpace;priorityClassName | Set PriorityClass, overrides global | `""` |
Expand Down
3 changes: 0 additions & 3 deletions chart/charts/crds/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,4 @@ A Helm chart that collects CustomResourceDefinitions (CRDs) from Mayastor.
| csi.&ZeroWidthSpace;volumeSnapshots.&ZeroWidthSpace;annotations | Annotations to be added to all CRDs | <pre>{<br><br>}</pre> |
| csi.&ZeroWidthSpace;volumeSnapshots.&ZeroWidthSpace;enabled | Install Volume Snapshot CRDs | `true` |
| csi.&ZeroWidthSpace;volumeSnapshots.&ZeroWidthSpace;keep | Keep CRDs on chart uninstall | `true` |
| jaeger.&ZeroWidthSpace;annotations | Annotations to be added to all CRDs | <pre>{<br><br>}</pre> |
| jaeger.&ZeroWidthSpace;enabled | Install Jaeger CRDs | `true` |
| jaeger.&ZeroWidthSpace;keep | Keep CRDs on chart uninstall | `true` |

50 changes: 0 additions & 50 deletions chart/charts/crds/templates/jaeger.yaml

This file was deleted.

11 changes: 0 additions & 11 deletions chart/charts/crds/values.yaml
Original file line number Diff line number Diff line change
@@ -1,13 +1,3 @@
jaeger:
# -- Install Jaeger CRDs
enabled: true
# -- Keep CRDs on chart uninstall
keep: true
# -- Annotations to be added to all CRDs
annotations: {}
# Example for Argo CD to prevent CRDs from being recycled
# argocd.argoproj.io/sync-options: Prune=false

csi:
volumeSnapshots:
# -- Install Volume Snapshot CRDs
Expand All @@ -18,4 +8,3 @@ csi:
annotations: {}
# Example for Argo CD to prevent CRDs from being recycled
# argocd.argoproj.io/sync-options: Prune=false

4 changes: 2 additions & 2 deletions chart/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ Usage:
{{ include "csi_node_init_containers" . }}
*/}}
{{- define "csi_node_init_containers" -}}
{{- if .Values.csi.node.initContainers.enabled }}
{{- if (.Values.csi.node.initContainers).enabled }}
{{- include "render" (dict "value" .Values.csi.node.initContainers.containers "context" $) | nindent 8 }}
{{- end }}
{{- end -}}
Expand Down Expand Up @@ -250,4 +250,4 @@ Usage:
{{ $param | quote }}: {{ $val | quote }}
{{- end -}}
{{- end -}}
{{- end -}}
{{- end -}}
2 changes: 1 addition & 1 deletion chart/templates/mayastor/csi/csi-node-daemonset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ spec:
fieldPath: status.podIP
- name: RUST_BACKTRACE
value: "1"
{{- if .Values.csi.node.mkfs_args.xfs }}
{{- if (.Values.csi.node.mkfs_args).xfs }}
- name: MKFS_XFS_ARGS
value: {{ .Values.csi.node.mkfs_args.xfs | quote }}
{{- end }}
Expand Down
10 changes: 3 additions & 7 deletions chart/values.yaml
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
crds:
# -- Disables the installation of all CRDs if set to false
enabled: true
jaeger:
# -- Install Jaeger CRDs
enabled: true
csi:
volumeSnapshots:
# -- Install Volume Snapshot CRDs
Expand Down Expand Up @@ -103,7 +100,9 @@ base:
enabled: true

jaeger:
# -- Enable jaeger tracing
# Enable jaeger tracing (for development only).
# Since version 1.31 the Jaeger Operator uses webhooks to validate Jaeger custom resources (CRs).
# This requires an installed version of the cert-manager.
enabled: false
initContainer: true
agent:
Expand Down Expand Up @@ -143,9 +142,6 @@ operators:
jaeger-operator:
# Name of jaeger operator
name: "{{ .Release.Name }}"
crd:
# Install jaeger CRDs
install: false
jaeger:
# Install jaeger-operator
create: false
Expand Down
16 changes: 9 additions & 7 deletions scripts/release.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
CI=${CI-}

set -euo pipefail
trap 'cleanup_and_exit "$?"' EXIT

# Test if the image already exists in dockerhub
dockerhub_tag_exists() {
curl --silent -f -lSL https://hub.docker.com/v2/repositories/$1/tags/$2 1>/dev/null 2>&1
Expand All @@ -34,7 +34,7 @@ cleanup_and_exit() {
local -r status=${1}

# Remove helm subcharts, if `helm dependency update` was run.
if [ "$_helm_dependencies_updated" = "true" ]; then
if [ "$HELM_DEPS_UPDATED" = "true" ]; then
echo "Cleaning up helm chart dependencies..."
for dep_chart in "$CHART_DIR"/charts/*; do
if [ "$dep_chart" = "$CHART_DIR/charts/crds" ]; then
Expand Down Expand Up @@ -99,6 +99,9 @@ DEFAULT_BINARIES="kubectl-plugin"
BUILD_BINARIES=
BIN_TARGET_PLAT="linux-musl"
BINARY_OUT_LINK="."
# This variable will be used to flag if the helm chart dependencies have been
# been updated.
HELM_DEPS_UPDATED="false"

# Check if all needed tools are installed
curl --version >/dev/null
Expand Down Expand Up @@ -192,6 +195,8 @@ while [ "$#" -gt 0 ]; do
esac
done

trap 'cleanup_and_exit "$?"' EXIT

cd $SCRIPTDIR/..

if [ -z "$IMAGES" ]; then
Expand Down Expand Up @@ -243,9 +248,6 @@ if [ -n "$BUILD_BINARIES" ]; then
done
fi

# This variable will be used to flag if the helm chart dependencies have been
# been updated.
_helm_dependencies_updated="false"
for name in $IMAGES; do
image_basename=$($NIX_EVAL -f . images.$BUILD_TYPE.$name.imageName | xargs)
image=$image_basename
Expand All @@ -257,7 +259,7 @@ for name in $IMAGES; do
fi
fi

if [ "$_helm_dependencies_updated" = "false" ]; then
if [ "$HELM_DEPS_UPDATED" = "false" ]; then
for helm_chart_user in ${IMAGES_THAT_REQUIRE_HELM_CHART[@]}; do
if [ "$name" = "$helm_chart_user" ]; then
echo "Updating helm chart dependencies..."
Expand All @@ -276,7 +278,7 @@ for name in $IMAGES; do
done

# Set flag to true
_helm_dependencies_updated="true"
HELM_DEPS_UPDATED="true"
break
fi
done
Expand Down

0 comments on commit ccc8b9f

Please sign in to comment.