Skip to content

Conversation

marcuz
Copy link
Contributor

@marcuz marcuz commented Feb 7, 2023

Proposed changes

Fixes error:

controller.topologySpreadConstraints: Invalid type. Expected: object, given: array

Expected YAML:

controller:
  topologySpreadConstraints:
    - maxSkew: 1
      topologyKey: topology.kubernetes.io/zone
      whenUnsatisfiable: DoNotSchedule
      labelSelector:
        matchLabels:
          app: nginx-ingress
    - maxSkew: 1
      topologyKey: kubernetes.io/hostname
      whenUnsatisfiable: DoNotSchedule
      labelSelector:
        matchLabels:
          app: nginx-ingress

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

@marcuz marcuz requested a review from a team as a code owner February 7, 2023 11:49
@github-actions github-actions bot added documentation Pull requests/issues for documentation helm_chart Pull requests that update the Helm Chart labels Feb 7, 2023
@codecov-commenter
Copy link

codecov-commenter commented Feb 7, 2023

Codecov Report

Merging #3527 (49a6be2) into main (9b435ad) will increase coverage by 0.02%.
The diff coverage is n/a.

❗ Current head 49a6be2 differs from pull request most recent head 4fb481f. Consider uploading reports for the commit 4fb481f to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main    #3527      +/-   ##
==========================================
+ Coverage   51.98%   52.00%   +0.02%     
==========================================
  Files          60       60              
  Lines       16816    16816              
==========================================
+ Hits         8742     8746       +4     
+ Misses       7777     7775       -2     
+ Partials      297      295       -2     
Impacted Files Coverage Δ
internal/k8s/configuration.go 95.79% <0.00%> (+0.36%) ⬆️

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

Copy link

@lucacome lucacome left a comment

Choose a reason for hiding this comment

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

Hi @marcuz I think we just need to change the ref to the right definition and everything else can remain the same

@lucacome lucacome self-assigned this Feb 7, 2023
@marcuz
Copy link
Contributor Author

marcuz commented Feb 8, 2023

@lucacome sorry I totally misunderstood the behaviour of $ref here. Pushing a cleanup commit soon!

@vepatel vepatel self-requested a review February 8, 2023 09:57
@github-actions github-actions bot removed the documentation Pull requests/issues for documentation label Feb 8, 2023
@marcuz
Copy link
Contributor Author

marcuz commented Feb 8, 2023

Just changing the $ref as suggested, lint passes successfully:

$ helm lint . -f values.yaml -f values-test.yaml 
==> Linting .

1 chart(s) linted, 0 chart(s) failed

values-test.yaml contains the controller.topologySpreadConstraints as specified above.

Copy link

@lucacome lucacome left a comment

Choose a reason for hiding this comment

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

Thanks @marcuz !

@lucacome lucacome merged commit 27973bf into nginx:main Feb 9, 2023
@marcuz
Copy link
Contributor Author

marcuz commented Feb 9, 2023

Thanks @marcuz !

Sorry for all the back and forth. 😄

@marcuz marcuz deleted the fix-tsc branch February 9, 2023 08:16
@cpuengr949
Copy link

cpuengr949 commented Feb 10, 2023

@lucacome I am getting this same error using terraform to build
topologySpreadConstraints = [
{
labelSelector = {
matchLabels = {
app = "nginx-plus-nginx-ingress"
}
}
maxSkew = 1
topologyKey = "kubernetes.io/hostname"
whenUnsatisfiable = "DoNotSchedule"
}
]

Which is creating the code
"topologySpreadConstraints":
- "labelSelector":
"matchLabels":
"app": "nginx-plus-nginx-ingress"
"maxSkew": 1
"topologyKey": "kubernetes.io/hostname"
"whenUnsatisfiable": "DoNotSchedule"

@lucacome
Copy link

@cpuengr949 even with the fix in this PR?

@cpuengr949
Copy link

@lucacome How do I get the fix?
I believe terraform downloads it each time right?
Thank You

@lucacome
Copy link

@cpuengr949 if it's pulling the main branch from here or using the edge helm chart, it should have the fix in this PR, but I don't know what your setup is 🙂

lucacome pushed a commit that referenced this pull request Feb 13, 2023
Signed-off-by: Marco Londero <[email protected]>
(cherry picked from commit 27973bf)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

helm_chart Pull requests that update the Helm Chart

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants