SoftwareAG / webmethods-helm-charts

This repository contains a collection of Helm charts for various webMethods components.
https://open-source.softwareag.com/webmethods-helm-charts/
Apache License 2.0
9 stars 14 forks source link

Merge main into terracottabm + Fix Terrarcotta BM Helm Charts #50

Closed mathieucarbou closed 7 months ago

mathieucarbou commented 7 months ago

This PR contains:

As requested, can you please give Haque and me access to this repo in RW so that we can correctly and efficiently maintain our Helm Charts. We need to:

Thanks.

mathieucarbou commented 7 months ago

@MarcFriedhoff : the current helm charts in this PR pass our smoke tests without security enabled.

They fail when security is enabled.

The reason is that you have changed the service names and statefulset names by prepending {{ include "common.names.fullname" . }}.

This has the consequence of changing the hostnames of the servers, their names also, and pod names, and thus how security works.

We initially didn't want for the user to be able to change the internal k8s assigned named for services and statefulset. The reason is that changing that changes all the security config of the user, and also the tc config file.

The problem comes when a user needs to replicate for example a tc cluster somewhere else. He needs to be able to keep the same config files and data and move and without any server name, pod name change or service name change.

The change you did will also break the compatibility with an existing deployment in terms of tc-config files and security config files.

So I will rollback your changes.

If you can show us a really good reason to do that, which can be a blocker regarding k8s deployment for example, then tell us.

But if it is just for convention purposes or to make things beautiful, then, this change is not acceptable.

I will add a commit on top of this one to isolate this rollback from the rest.

MarcFriedhoff commented 7 months ago

Hi Mathieu,

actually there was a good reason – but you could argue that this will not be the case for TC: what if you deploy multiple TCs in the same namespace. At least for the webMethods components we see customers having this requirement…

Regards,

Marc

From: Mathieu Carbou @.> Date: Thursday, 29. February 2024 at 13:24 To: SoftwareAG/webmethods-helm-charts @.> Cc: Friedhoff, Marc @.>, Mention @.> Subject: Re: [SoftwareAG/webmethods-helm-charts] Merge main into terracottabm + Fix Terrarcotta BM Helm Charts (PR #50)

@MarcFriedhoffhttps://github.com/MarcFriedhoff : the current helm charts in this PR pass our smoke tests without security enabled.

They fail when security is enabled.

The reason is that you have changed the service names and statefulset names by prepending {{ include "common.names.fullname" . }}.

This has the consequence of changing the hostnames of the servers, their names also, and pod names, and thus how security works.

We initially didn't want for the user to be able to change the internal k8s assigned named for services and statefulset. The reason is that changing that changes all the security config of the user, and also the tc config file.

The problem comes when a user needs to replicate for example a tc cluster somewhere else. He needs to be able to keep the same config files and data and move and without any server name, pod name change or service name change.

So I will rollback your changes.

If you can show us a really good reason to do that, which can be a blocker regarding k8s deployment for example, then tell us.

But if it is just for convention purposes or to make things beautiful, then, this change is not acceptable.

— Reply to this email directly, view it on GitHubhttps://github.com/SoftwareAG/webmethods-helm-charts/pull/50#issuecomment-1971027464, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AHCQXLN4TOZW7ESK55GTASTYV4OWHAVCNFSM6AAAAABD7QWVR2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZRGAZDONBWGQ. You are receiving this because you were mentioned.Message ID: @.***>

SAG Consulting Services GmbH - Sitz/Registered office: Uhlandstraße 9, 64297 Darmstadt, Germany - Registergericht/Commercial register: Darmstadt HRB 85598 Geschäftsführer/Managing Directors: Ralf Stohldreier, Thomas Alberti - http://www.softwareag.com http://www.softwareag.com/

mathieucarbou commented 7 months ago

Hi Mathieu, actually there was a good reason – but you could argue that this will not be the case for TC: what if you deploy multiple TCs in the same namespace. At least for the webMethods components we see customers having this requirement…

Ok! Yes, for a TC cluster it does not make sense: a multi-stripe TC cluster with active/passives on each stripes has to be isolated from each other for several good reasons.

mathieucarbou commented 7 months ago

@MarcFriedhoff : PR is complete and smoke tests are passing.