Skip to content

Move logic for -enable-leader-election flag in helm templates #3475

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

Merged
merged 7 commits into from
Jan 24, 2023

Conversation

shaun-nx
Copy link
Contributor

@shaun-nx shaun-nx commented Jan 24, 2023

Proposed changes

This change ensures the -enable-leader-election flag it set to false when controller.reportIngressStatus.enable is set to false.

Closes #3455

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@shaun-nx shaun-nx requested a review from a team as a code owner January 24, 2023 12:32
@github-actions github-actions bot added bug An issue reporting a potential bug helm_chart Pull requests that update the Helm Chart labels Jan 24, 2023
@shaun-nx shaun-nx changed the title Move logic for -enable-leader-election flag Move logic for -enable-leader-election flag in helm templates Jan 24, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jan 24, 2023

Codecov Report

Merging #3475 (aa68f8f) into main (3637ddd) will increase coverage by 0.04%.
The diff coverage is 100.00%.

❗ Current head aa68f8f differs from pull request most recent head 756d550. Consider uploading reports for the commit 756d550 to get more accurate results

@@            Coverage Diff             @@
##             main    #3475      +/-   ##
==========================================
+ Coverage   51.95%   51.99%   +0.04%     
==========================================
  Files          60       60              
  Lines       16797    16811      +14     
==========================================
+ Hits         8727     8741      +14     
  Misses       7775     7775              
  Partials      295      295              
Impacted Files Coverage Δ
internal/configs/version2/http.go 0.00% <ø> (ø)
pkg/apis/configuration/validation/policy.go 91.16% <ø> (ø)
internal/configs/virtualserver.go 95.07% <100.00%> (+0.01%) ⬆️
internal/k8s/configuration.go 95.79% <100.00%> (+0.03%) ⬆️
internal/k8s/validation.go 93.81% <100.00%> (ø)
...is/configuration/validation/globalconfiguration.go 88.33% <100.00%> (ø)
...g/apis/configuration/validation/transportserver.go 96.62% <100.00%> (ø)
pkg/apis/configuration/validation/virtualserver.go 94.39% <100.00%> (+<0.01%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@ciarams87 ciarams87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to modify deployments/helm-chart/templates/controller-leader-election-configmap.yaml as well so that file is only created when the leader election is set to true.

e.g.

{{- if .Values.controller.reportIngressStatus.enableLeaderElection }}
apiVersion: v1
kind: ConfigMap
metadata:
  name: {{ include "nginx-ingress.leaderElectionName" . }}
  namespace: {{ .Release.Namespace }}
  labels:
    {{- include "nginx-ingress.labels" . | nindent 4 }}
{{- if .Values.controller.reportIngressStatus.annotations }}
  annotations:
{{ toYaml .Values.controller.reportIngressStatus.annotations | indent 4 }}
{{- end }}
{{- end }}

@shaun-nx shaun-nx requested a review from ciarams87 January 24, 2023 14:09
@shaun-nx shaun-nx merged commit ae5caa5 into main Jan 24, 2023
@shaun-nx shaun-nx deleted the bug/set-leader-election branch January 24, 2023 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue reporting a potential bug helm_chart Pull requests that update the Helm Chart
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disabling Leader Election in helm chart
4 participants