Skip to content
This repository has been archived by the owner on Feb 22, 2022. It is now read-only.

[stable/unifi] unifi chart enhancements #12047

Merged
merged 8 commits into from
Mar 28, 2019
Merged

[stable/unifi] unifi chart enhancements #12047

merged 8 commits into from
Mar 28, 2019

Conversation

billimek
Copy link
Collaborator

@billimek billimek commented Mar 10, 2019 •
edited
Loading

What this PR does / why we need it:

  • based on the persistent nature of this chart, using a default strategy Type of 'Recreate' instead of 'RollingUpdate' - with an optional override
  • bumping unifi controller to the latest stable version (5.10.19)
  • adding @mcronce to the OWNERS file
  • updating label syntax to current helm standards (e.g.
    app.kubernetes.io/name)

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • DCO signed
  • Chart Version bumped
  • Variables are documented in the README.md

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 10, 2019
@helm-bot helm-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Mar 10, 2019
@billimek billimek force-pushed the unifi-statefulset branch from 49f6a0a to 637f2a5 Compare March 10, 2019 14:20
@helm-bot helm-bot added Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 10, 2019
@billimek
Copy link
Collaborator Author

FYI @mcronce, adding you to the OWNERS file as part of this PR

@billimek
Copy link
Collaborator Author

Going to make some additional changes and re-submit

@billimek billimek closed this Mar 10, 2019
@billimek billimek reopened this Mar 10, 2019
@helm-bot helm-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 10, 2019
@billimek
Copy link
Collaborator Author

Re-opening after reviewing the StatefulSet section of the helm chart guidelines

@helm-bot helm-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 10, 2019
@billimek billimek force-pushed the unifi-statefulset branch from 44c48be to d3bbf6f Compare March 10, 2019 20:28
@helm-bot helm-bot added Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 10, 2019
@mcronce
Copy link
Collaborator

mcronce commented Mar 12, 2019

@billimek Received 👍

@helm-bot helm-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 12, 2019
@billimek
Copy link
Collaborator Author

/assign @tompizmor

@billimek
Copy link
Collaborator Author

/assign @unguiculus

@billimek
Copy link
Collaborator Author

/assign @davidkarlsen

@billimek
Copy link
Collaborator Author

Hi friends, thank you so much for the really great discussion on this topic!

@unguiculus your amazing attention to detail is honestly really appreciated - thank you!

@davidkarlsen, @scottrigby, thanks for the additional feedback as well - this clarifies some misconceptions I had about when to use statefulsets.

Based on this discussion I'll update this PR (and the other two similar charts) to:

  • Use Deployment instead of StatefulSet as is the current mode
  • Default the upgrade strategy to be Recreate instead of RollingUpdate with the option to change this via a values parameter
  • Bump minor version instead of major given that it won't be a breaking change any more

@helm-bot helm-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). labels Mar 26, 2019
@billimek
revert to Deployment and leverage strategy types
1523b5b 
Signed-off-by: Jeff Billimek <jeff@billimek.com>
@billimek billimek force-pushed the unifi-statefulset branch from fd671ac to 1523b5b Compare March 26, 2019 02:22
@helm-bot helm-bot added the Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). label Mar 26, 2019
@billimek
include readme entry for strategyType
8552b96 
Signed-off-by: Jeff Billimek <jeff@billimek.com>
@billimek billimek changed the title [stable/unifi] switching unifi chart to SatefulSet [stable/unifi] unifi chart enhancements Mar 26, 2019
@billimek
Copy link
Collaborator Author

@unguiculus & @davidkarlsen these changes are made, FYI!

unguiculus
unguiculus suggested changes Mar 27, 2019
View reviewed changes
Copy link
Member

@unguiculus unguiculus left a comment

Choose a reason for hiding this comment

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

The number of replicas should not longer be configurable.

stable/unifi/OWNERS Show resolved Hide resolved
@billimek
hard-code replica count and add mcronce to Chart maintainers
d71fa14 
Signed-off-by: Jeff Billimek <jeff@billimek.com>
@helm-bot helm-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 27, 2019
@billimek
fixing linting error
276661f 
Signed-off-by: Jeff Billimek <jeff@billimek.com>
@unguiculus
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 28, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: billimek, unguiculus

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@unguiculus
Copy link
Member

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 28, 2019
@k8s-ci-robot k8s-ci-robot merged commit 5f03f4e into helm:master Mar 28, 2019
@billimek billimek deleted the unifi-statefulset branch March 29, 2019 01:07
crackmac pushed a commit to crackmac/charts that referenced this pull request Mar 29, 2019
@billimek
[stable/unifi] unifi chart enhancements (helm#12047)
df4d9d8 
* switching unifi chart to SatefulSet

* based on the persistent nature of this chart as well as [this
discussion](helm#1863), migrating the
chart to a StatefulSet instead of a deployment. As a result bumping the
major version
* bumping unifi controller to the latest stable version (5.10.19)
* adding @mcronce to the OWNERS file

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* using volumeClaimTemplates for statefulSet

* also updating label syntax to current helm standards (e.g.
`app.kubernetes.io/name`)

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* fixing indenting

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* using Parallel podManagementPolicy

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* revert to Deployment and leverage strategy types

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* include readme entry for strategyType

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* hard-code replica count and add mcronce to Chart maintainers

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* fixing linting error

Signed-off-by: Jeff Billimek <jeff@billimek.com>
crackmac pushed a commit to crackmac/charts that referenced this pull request Mar 29, 2019
@billimek
[stable/unifi] unifi chart enhancements (helm#12047)
35fdceb 
* switching unifi chart to SatefulSet

* based on the persistent nature of this chart as well as [this
discussion](helm#1863), migrating the
chart to a StatefulSet instead of a deployment. As a result bumping the
major version
* bumping unifi controller to the latest stable version (5.10.19)
* adding @mcronce to the OWNERS file

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* using volumeClaimTemplates for statefulSet

* also updating label syntax to current helm standards (e.g.
`app.kubernetes.io/name`)

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* fixing indenting

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* using Parallel podManagementPolicy

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* revert to Deployment and leverage strategy types

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* include readme entry for strategyType

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* hard-code replica count and add mcronce to Chart maintainers

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* fixing linting error

Signed-off-by: Jeff Billimek <jeff@billimek.com>
Signed-off-by: Kevin Duane <duank001@apps.disney.com>
devnulled pushed a commit to devnulled/charts that referenced this pull request Apr 25, 2019
@billimek @devnulled
[stable/unifi] unifi chart enhancements (helm#12047)
8a1c180 
* switching unifi chart to SatefulSet

* based on the persistent nature of this chart as well as [this
discussion](helm#1863), migrating the
chart to a StatefulSet instead of a deployment. As a result bumping the
major version
* bumping unifi controller to the latest stable version (5.10.19)
* adding @mcronce to the OWNERS file

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* using volumeClaimTemplates for statefulSet

* also updating label syntax to current helm standards (e.g.
`app.kubernetes.io/name`)

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* fixing indenting

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* using Parallel podManagementPolicy

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* revert to Deployment and leverage strategy types

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* include readme entry for strategyType

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* hard-code replica count and add mcronce to Chart maintainers

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* fixing linting error

Signed-off-by: Jeff Billimek <jeff@billimek.com>
dermorz pushed a commit to dermorz/charts that referenced this pull request Apr 26, 2019
@billimek
[stable/unifi] unifi chart enhancements (helm#12047)
58e7fa5 
* switching unifi chart to SatefulSet

* based on the persistent nature of this chart as well as [this
discussion](helm#1863), migrating the
chart to a StatefulSet instead of a deployment. As a result bumping the
major version
* bumping unifi controller to the latest stable version (5.10.19)
* adding @mcronce to the OWNERS file

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* using volumeClaimTemplates for statefulSet

* also updating label syntax to current helm standards (e.g.
`app.kubernetes.io/name`)

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* fixing indenting

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* using Parallel podManagementPolicy

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* revert to Deployment and leverage strategy types

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* include readme entry for strategyType

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* hard-code replica count and add mcronce to Chart maintainers

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* fixing linting error

Signed-off-by: Jeff Billimek <jeff@billimek.com>
@billimek billimek mentioned this pull request May 4, 2019
Closed
3 tasks
goshlanguage pushed a commit to goshlanguage/charts that referenced this pull request May 17, 2019
@billimek @goshlanguage
[stable/unifi] unifi chart enhancements (helm#12047)
be87b7d 
* switching unifi chart to SatefulSet

* based on the persistent nature of this chart as well as [this
discussion](helm#1863), migrating the
chart to a StatefulSet instead of a deployment. As a result bumping the
major version
* bumping unifi controller to the latest stable version (5.10.19)
* adding @mcronce to the OWNERS file

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* using volumeClaimTemplates for statefulSet

* also updating label syntax to current helm standards (e.g.
`app.kubernetes.io/name`)

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* fixing indenting

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* using Parallel podManagementPolicy

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* revert to Deployment and leverage strategy types

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* include readme entry for strategyType

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* hard-code replica count and add mcronce to Chart maintainers

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* fixing linting error

Signed-off-by: Jeff Billimek <jeff@billimek.com>
eyenx pushed a commit to eyenx/charts that referenced this pull request May 28, 2019
@billimek @eyenx
[stable/unifi] unifi chart enhancements (helm#12047)
1ec6672 
* switching unifi chart to SatefulSet

* based on the persistent nature of this chart as well as [this
discussion](helm#1863), migrating the
chart to a StatefulSet instead of a deployment. As a result bumping the
major version
* bumping unifi controller to the latest stable version (5.10.19)
* adding @mcronce to the OWNERS file

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* using volumeClaimTemplates for statefulSet

* also updating label syntax to current helm standards (e.g.
`app.kubernetes.io/name`)

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* fixing indenting

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* using Parallel podManagementPolicy

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* revert to Deployment and leverage strategy types

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* include readme entry for strategyType

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* hard-code replica count and add mcronce to Chart maintainers

Signed-off-by: Jeff Billimek <jeff@billimek.com>

* fixing linting error

Signed-off-by: Jeff Billimek <jeff@billimek.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@davidkarlsen davidkarlsen davidkarlsen requested changes

@unguiculus unguiculus unguiculus approved these changes

@foxish foxish Awaiting requested review from foxish

@sameersbn sameersbn Awaiting requested review from sameersbn

Assignees

@davidkarlsen davidkarlsen

@unguiculus unguiculus

@tompizmor tompizmor

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@billimek @mcronce @unguiculus @davidkarlsen @scottrigby @k8s-ci-robot @tompizmor @helm-bot