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.

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

Bump nginx to v1.10.1 #6022

Merged
merged 1 commit into from
May 28, 2024
Merged

Bump nginx to v1.10.1 #6022

merged 1 commit into from
May 28, 2024

Conversation

dereknola
Copy link
Contributor

@dereknola dereknola commented May 28, 2024

Proposed Changes

  • Chart bump

Verification

Use https://github.com/esigo/nginx-example and the following manifest to validate Opentelemetry support:

apiVersion: helm.cattle.io/v1
kind: HelmChartConfig
metadata:
  name: rke2-ingress-nginx
  namespace: kube-system
spec:
  valuesContent: |-
    controller:
      opentelemetry:
        enabled: true
        containerSecurityContext:
          allowPrivilegeEscalation: false
      config:
        enable-opentelemetry: true
        opentelemetry-config: /etc/nginx/opentelemetry.toml
        opentelemetry-operation-name: HTTP $request_method $service_name $uri
        opentelemetry-trust-incoming-span: 'true'
        otlp-collector-host: "otel-coll-collector.otel.svc"
        otlp-collector-port: '4317'
        otel-max-queuesize: '2048'
        otel-schedule-delay-millis: '5000'
        otel-max-export-batch-size: '512'
        otel-service-name: nginx-proxy
        otel-sampler: AlwaysOn
        otel-sampler-ratio: '1.0'
        otel-sampler-parent-based: false

Based of a modified version of https://kubernetes.github.io/ingress-nginx/user-guide/third-party-addons/opentelemetry/

helm repo add open-telemetry https://open-telemetry.github.io/opentelemetry-helm-charts
helm repo add grafana https://grafana.github.io/helm-charts
helm repo update
kubectl apply -f https://github.com/cert-manager/cert-manager/releases/download/v1.9.1/cert-manager.yaml
kubectl apply -f https://raw.githubusercontent.com/esigo/nginx-example/main/observability/namespace.yaml
helm upgrade --install otel-collector-operator -n otel --create-namespace open-telemetry/opentelemetry-operator --set "manager.collectorImage.repository=otel/opentelemetry-collector-k8s"
helm upgrade --install tempo grafana/tempo --create-namespace -n observability
helm upgrade -f https://raw.githubusercontent.com/esigo/nginx-example/main/observability/grafana/grafana-values.yaml --install grafana grafana/grafana --create-namespace -n observability

Pull down the https://github.com/esigo/nginx-example repo and modify the collector:

git clone https://github.com/esigo/nginx-example.git
cd nginx-example/

Modify observability/collector.yaml and remove all mention of jaeger and zipkin, leave just otlp exporter, then apply

kubectl apply -f ./observability/collector.yaml

Then run:

make images
docker save service-a service-b -o service.tar
mv ./service.tar /var/lib/rancher/rke2/agent/images/service.tar
systemctl restart rke2-server
make deploy-app

Then access grafana dashboard

kubectl port-forward --namespace=observability service/grafana 3000:80

Linked Issues

#5539

User-Facing Change


Further Comments

Signed-off-by: Derek Nola <[email protected]>
@dereknola dereknola requested a review from a team as a code owner May 28, 2024 16:00
@codecov-commenter
Copy link

codecov-commenter commented May 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 26.20%. Comparing base (d2cae59) to head (3f98d32).

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #6022       +/-   ##
===========================================
+ Coverage    9.86%   26.20%   +16.34%     
===========================================
  Files          32       32               
  Lines        2686     2686               
===========================================
+ Hits          265      704      +439     
+ Misses       2399     1936      -463     
- Partials       22       46       +24     
Flag Coverage Δ
inttests 9.86% <ø> (ø)
unittests 18.65% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@albundy83
Copy link

Hello,

I have checked using the release v1.27.15-rc3+rke2r1 and by overriding rke2-ingress-nginx using an HelmChartConfig object:

apiVersion: helm.cattle.io/v1
kind: HelmChartConfig
metadata:
  name: rke2-ingress-nginx
  namespace: kube-system
spec:
  bootstrap: true
  valuesContent: |
    controller:
      opentelemetry:
        enabled: true
        containerSecurityContext:
          allowPrivilegeEscalation: false
      config:
        enable-opentelemetry: true
        opentelemetry-config: /etc/nginx/opentelemetry.toml
        opentelemetry-operation-name: HTTP $request_method $service_name $uri
        opentelemetry-trust-incoming-span: 'true'
        otlp-collector-host: tempo.tempo # or otel-coll-collector.otel.svc
        otlp-collector-port: '4317'
        otel-max-queuesize: '2048'
        otel-schedule-delay-millis: '5000'
        otel-max-export-batch-size: '512'
        otel-service-name: nginx-proxy
        otel-sampler: AlwaysOn
        otel-sampler-ratio: '1.0'
        otel-sampler-parent-based: false
        server-snippet: |
          opentelemetry_attribute "ingress.namespace" "$namespace";
          opentelemetry_attribute "ingress.service_name" "$service_name";
          opentelemetry_attribute "ingress.name" "$ingress_name";
          opentelemetry_attribute "ingress.upstream" "$proxy_upstream_name";

It has worked perfectly, thanks a lot !!! 👍

By the way @dereknola, in your example here, you did not indent correctly allowPrivilegeEscalation: false

...
spec:
  valuesContent: |-
    controller:
      opentelemetry:
        enabled: true
        containerSecurityContext:
        allowPrivilegeEscalation: false
...

Maybe you could edit your comment and change it to:

...
spec:
  valuesContent: |-
    controller:
      opentelemetry:
        enabled: true
        containerSecurityContext:
          allowPrivilegeEscalation: false
...

@dereknola
Copy link
Contributor Author

Fixed! Thanks for validating the feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants