restate icon indicating copy to clipboard operation
restate copied to clipboard

Split Ingress role from worker role

Open tillrohrmann opened this issue 8 months ago • 3 comments

Split Ingress role from worker role: https://github.com/restatedev/restate/blob/98204e028638b0348b79a0bdd06aff9cbd0cf7c5/crates/types/src/nodes_config.rs#L52

tillrohrmann avatar Apr 10 '25 08:04 tillrohrmann

@tillrohrmann Could you please add some detail of this feature?I would like to take this. for example, if we only make ingress_role GA and removed all validation on experimental_feature_enable_separate_ingress_role, is it enough?

lsytj0413 avatar Apr 11 '25 02:04 lsytj0413

Thanks for offering to pick this issue up @lsytj0413. Yes, the idea would be to remove the implicit coupling of Role::Worker and Role::HttpIngress. So instead of activating the HttpIngress whenever the Worker role is specified, it needs to be specified explicitly. The default set of roles defined in CommonOptions should then include the HttpIngress. The challenge will be to notify people that have set the roles explicitly and rely on Worker starting the ingress role when they migrate from a previous version because all of a sudden they wouldn't start the ingress.

Since this change will require that we release the next minor version (1.4.0), we need to wait with merging a PR for this change until we no longer want to release 1.3.x bug fix releases. Otherwise we need to cut a release branch and maintain multiple branches.

tillrohrmann avatar Apr 11 '25 07:04 tillrohrmann

Thanks for offering to pick this issue up @lsytj0413. Yes, the idea would be to remove the implicit coupling of Role::Worker and Role::HttpIngress. So instead of activating the HttpIngress whenever the Worker role is specified, it needs to be specified explicitly. The default set of roles defined in CommonOptions should then include the HttpIngress. The challenge will be to notify people that have set the roles explicitly and rely on Worker starting the ingress role when they migrate from a previous version because all of a sudden they wouldn't start the ingress.

Since this change will require that we release the next minor version (1.4.0), we need to wait with merging a PR for this change until we no longer want to release 1.3.x bug fix releases. Otherwise we need to cut a release branch and maintain multiple branches.

There may be the following situations during the upgrade:

  1. They have used the default settings, and after upgrade Worker & HttpIngress will start, so this should be compatible.
  2. They explicitly enable Worker and HttpIngress role, this should be compatible. (even if experimental_feature_enable_separate_ingress_role is dropped)
  3. They explicitly enable Worker, but didn't enbale HttpIngress role, after upgrade HttpIngress will not start, this may disrupt the functions that were originally running properly.

In the third situation mentioned above, I didn't think of a way to automatically achieve compatibility through code (because we didn't record which roles were launched previously). Do you have any other suggestions?

lsytj0413 avatar Apr 12 '25 04:04 lsytj0413

Implemented in https://github.com/restatedev/restate/pull/3443 but in v1.5.0 we need to make http-ingress the only signal needed to start the ingress component.

AhmedSoliman avatar Jun 23 '25 15:06 AhmedSoliman