lunny/helm-chart
1
0
Fork 0
Code Issues 36 Pull Requests 14 Actions Packages Projects Releases 93 Wiki Activity

feat: service.{http,ssh}.loadBalancerClass #640

Merged
Karitham merged 5 commits from main into main 2024-07-15 15:13:25 +00:00
Conversation 2 Commits 5 Files Changed 5 +82 -1
Karitham commented 2024-04-15 15:56:12 +00:00 (Migrated from gitea.com)
Copy Link

Description of the change

Introduce service.{http,ssh}.loadBalancerClass

Benefits

Feature was not supported before. This is required if your cluster has multiple loadBalancer options and you want to select one

Possible drawbacks

More yaml.

Checklist

  • Parameters are documented in the values.yaml and added to the README.md using readme-generator-for-helm
  • Templating unittests are added
<!-- Before you open the request please review the following guidelines and tips to help it be more easily integrated: - Describe the scope of your change - i.e. what the change does. - Describe any known limitations with your change. - Please run any tests or examples that can exercise your modified code. Thank you for contributing! We will try to review, test and integrate the change as soon as we can. --> ### Description of the change Introduce `service.{http,ssh}.loadBalancerClass` <!-- Describe the scope of your change - i.e. what the change does. --> ### Benefits <!-- What benefits will be realized by the code change? --> Feature was not supported before. This is required if your cluster has multiple loadBalancer options and you want to select one ### Possible drawbacks <!-- Describe any known limitations with your change --> More yaml. ### Checklist <!-- [Place an '[X]' (no spaces) in all applicable fields. Please remove unrelated fields.] --> - [x] Parameters are documented in the `values.yaml` and added to the `README.md` using [readme-generator-for-helm](https://github.com/bitnami-labs/readme-generator-for-helm) - [x] Templating unittests are added
justusbunsi commented 2024-04-15 16:16:45 +00:00 (Migrated from gitea.com)
Copy Link

Thanks for contributing. It would be great to add unit tests for this feature. To ensure it is rendered correctly in different scenarios.

Thanks for contributing. It would be great to add unit tests for this feature. To ensure it is rendered correctly in different scenarios.
Karitham commented 2024-04-15 16:55:53 +00:00 (Migrated from gitea.com)
Copy Link

These tests should roughly cover the changes, namely making sure type: LoadBalancer properties aren't set when we aren't in a LoadBalancer and that loadBalancerClass exists (only) when it's set.

These tests should roughly cover the changes, namely making sure `type: LoadBalancer` properties aren't set when we aren't in a `LoadBalancer` and that `loadBalancerClass` exists (only) when it's set.
pat-s (Migrated from gitea.com) reviewed 2024-05-02 08:26:20 +00:00
unittests/deployment/svc-configuration.yaml
pat-s (Migrated from gitea.com) commented 2024-05-02 08:26:20 +00:00
Copy Link

Thanks for adding tests!

Can we expand the tests to check for existence/non-existence for both ssh and http?

Thanks for adding tests! Can we expand the tests to check for existence/non-existence for **both** `ssh` and `http`?
pat-s (Migrated from gitea.com) commented 2024-05-23 09:12:34 +00:00
Copy Link

ping @Karitham

ping @Karitham
pat-s (Migrated from gitea.com) commented 2024-05-27 07:20:52 +00:00
Copy Link

oop sorry forgot about this

oop sorry forgot about this
pat-s (Migrated from gitea.com) commented 2024-05-27 07:37:52 +00:00
Copy Link

I've added a test but I'm generally unhappy about using regex to test this, unfortunately I don't really have a better solution. Let me know if I can make them better

I've added a test but I'm generally unhappy about using regex to test this, unfortunately I don't really have a better solution. Let me know if I can make them better
pat-s (Migrated from gitea.com) commented 2024-05-30 19:36:12 +00:00
Copy Link

You could split the it with the regex and just duplicate that spec. Duplicating within tests is totally fine, IMO.

You could split the `it` with the regex and just duplicate that spec. Duplicating within tests is totally fine, IMO.
pat-s (Migrated from gitea.com) approved these changes 2024-07-13 12:04:50 +00:00
pat-s (Migrated from gitea.com) left a comment
Copy Link

@Karitham Thanks for your patience. LGTM now.

@justusbunsi Feel free to add more comments or merge!

@Karitham Thanks for your patience. LGTM now. @justusbunsi Feel free to add more comments or merge!
Sign in to join this conversation.
Reviewers
No Reviewers
pat-s
Labels
Clear labels
has/backport
in progress
invalid
kind/breaking
kind/bug
kind/build
kind/dependency
kind/deployment
kind/docs
kind/enhancement
kind/feature
kind/lint
kind/proposal
kind/question
kind/refactor
kind/security
kind/testing
kind/translation
kind/ui
need/backport
priority/critical
priority/low
priority/maybe
priority/medium
reviewed/duplicate
reviewed/invalid
reviewed/wontfix
skip-changelog
status/blocked
status/needs-feedback
status/needs-reviews
status/wip
upstream/gitea
upstream/other
No Label
kind/feature
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: lunny/helm-chart#640
No description provided.
Delete Branch "main"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?