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

Helm enhanchements #1174

Merged
merged 34 commits into from Dec 9, 2022
Merged

Helm enhanchements #1174

merged 34 commits into from Dec 9, 2022

Conversation

MichaelTrip
Copy link
Contributor

Hi there,

customer (@rduivenvoorde) requested for Frost to be implemented in their Kubernetes cluster. I have changed several things to the helm Chart.

For now i created the possibility to use an external Postgis / Postgres server. I will also include several other things:

  • The possibility to choose between the LoadBalancer of ClusterPort on the mqtt service./
  • The possibility to choose between http and https in the ingress for the http server.

I will try to add this to the PR later this week. For now, please don't merge it until i have added the LoadBalancer option and the ingress https option.

@hylkevds
Copy link
Member

Great, thanks for adding these features!

@MichaelTrip
Copy link
Contributor Author

PR is done;

  • Added the possibility to use tls on ingress without secretname. This can be handy when using cert-manager or when the ingress controller uses it's one, for example, wildcard certificate
  • Added the possibility to use annonations in the mqtt service. This can be handy when using a LoadBalancer service on premise such as metallb.
  • Added the possibility to use the serviceType of LoadBalancer instead of only using NodePort. This will be a breaking change bacause either LoadBalanceror NodePort must be specified. The default in values is NodePort.

@MichaelTrip MichaelTrip marked this pull request as ready for review September 23, 2022 08:18
@phertweck
Copy link
Contributor

Thank you very much for the contribution, and sorry for my late review.

I just have some small comments about the default behavior.

helm/frost-server/README.md Outdated Show resolved Hide resolved
helm/frost-server/README.md Outdated Show resolved Hide resolved
helm/frost-server/templates/mqtt-service.yaml Show resolved Hide resolved
helm/frost-server/values.yaml Outdated Show resolved Hide resolved
helm/frost-server/README.md Outdated Show resolved Hide resolved
helm/frost-server/values.yaml Outdated Show resolved Hide resolved
@phertweck
Copy link
Contributor

@MichaelTrip: Thanks for updating the PR! Looks fine for me :-)

Since the HELM chart also changed in v2.x-Branch, could you please rebase your branch? Afterwards, I can merge it.

@MichaelTrip
Copy link
Contributor Author

I fixed the merge conflicts. Should look fine now. Can you please have one final check? Sorry if something is off. Let's say that merge conflicts are not my core expertise ;-)

Michael Trip and others added 8 commits December 6, 2022 10:43
Bumps httpclient from 4.5.13 to 4.5.14.

---
updated-dependencies:
- dependency-name: org.apache.httpcomponents:httpclient
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [dependency-check-maven](https://github.com/jeremylong/DependencyCheck) from 7.3.1 to 7.4.0.
- [Release notes](https://github.com/jeremylong/DependencyCheck/releases)
- [Changelog](https://github.com/jeremylong/DependencyCheck/blob/main/CHANGELOG.md)
- [Commits](jeremylong/DependencyCheck@v7.3.1...v7.4.0)

---
updated-dependencies:
- dependency-name: org.owasp:dependency-check-maven
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [maven-dependency-plugin](https://github.com/apache/maven-dependency-plugin) from 3.3.0 to 3.4.0.
- [Release notes](https://github.com/apache/maven-dependency-plugin/releases)
- [Commits](apache/maven-dependency-plugin@maven-dependency-plugin-3.3.0...maven-dependency-plugin-3.4.0)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-dependency-plugin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
@MichaelTrip
Copy link
Contributor Author

Hi,

I tried to rebase it but i had a lot of merge conflicts. Can you please check if everything is ok? This was my first rebase ever, so please be gentle ;-)

Copy link
Member

@hylkevds hylkevds left a comment

Choose a reason for hiding this comment

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

The merge didn't go entirely correct, mainly because the MQTT section moved...

helm/frost-server/README.md Outdated Show resolved Hide resolved
helm/frost-server/README.md Outdated Show resolved Hide resolved
helm/frost-server/values.yaml Outdated Show resolved Hide resolved
@MichaelTrip
Copy link
Contributor Author

Hi Hylke,

I will try to fix this next friday

@MichaelTrip
Copy link
Contributor Author

Hi Hylke,

I changed the Helm chart with your comments. Can you please check again? Thanks!

@hylkevds hylkevds merged commit 129dfe2 into FraunhoferIOSB:v2.x Dec 9, 2022
@hylkevds
Copy link
Member

hylkevds commented Dec 9, 2022

Super! Thanks for the PR.

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

3 participants