Skip to content

Split Ingress role from worker role #3130

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

Open
tillrohrmann opened this issue Apr 10, 2025 · 3 comments · May be fixed by #3140
Open

Split Ingress role from worker role #3130

tillrohrmann opened this issue Apr 10, 2025 · 3 comments · May be fixed by #3140
Labels
release-blocker Blocker for the next release
Milestone

Comments

@tillrohrmann
Copy link
Contributor

Split Ingress role from worker role:

/// [EXPERIMENTAL FEATURE] Serves HTTP ingress requests (requires

@tillrohrmann tillrohrmann added the release-blocker Blocker for the next release label Apr 10, 2025
@tillrohrmann tillrohrmann added this to the 1.4 milestone Apr 10, 2025
@lsytj0413
Copy link
Contributor

@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?

@tillrohrmann
Copy link
Contributor Author

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.

@lsytj0413 lsytj0413 linked a pull request Apr 12, 2025 that will close this issue
@lsytj0413
Copy link
Contributor

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-blocker Blocker for the next release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants