Skip to content

[installer] change blobserve node to workload_ide #12558

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

Merged
merged 2 commits into from
Sep 9, 2022
Merged

Conversation

iQQBot
Copy link
Contributor

@iQQBot iQQBot commented Aug 31, 2022

Description

This PR move blobserve to workload_ide node, because traffic through ide-proxy

This PR also adjust blobserve network policy, in order to support some edge case see internal discuss

Related Issue(s)

Fixes #12366

How to test

Release Notes

NONE

Documentation

Werft options:

  • /werft with-preview

@iQQBot iQQBot changed the title Pd/fix-blobserve [installer] change blobserve node to workload_ide Aug 31, 2022
@roboquat roboquat added size/L and removed size/S labels Aug 31, 2022
@akosyakov
Copy link
Member

@iQQBot Should I fix it somehow?
or ask @gitpod-io/engineering-self-hosted to verify?

@iQQBot
Copy link
Contributor Author

iQQBot commented Sep 1, 2022

I think we should ask @gitpod-io/engineering-self-hosted to verify, i.e. use this version to reinstall

@akosyakov akosyakov requested a review from a team September 1, 2022 09:47
@github-actions github-actions bot added the team: delivery Issue belongs to the self-hosted team label Sep 1, 2022
Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

lgtm

/hold

please merge when you are comfortable

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-pd-fix-blobserve.2 because the annotations in the pull request description changed
(with .werft/ from main)

Copy link
Contributor

@mrsimonemms mrsimonemms left a comment

Choose a reason for hiding this comment

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

I'm broadly ok with this change, but I don't know the implications of changing the network policies. My recollection from when we did this is that it was only implemented in GKE (not EKS, AKS or k3s).

It's worth running the test suites we have. You'll need to load this PR in a workspace, and then run:

werft run github -j .werft/gke-installer-tests.yaml -a skipTests=true
werft run github -j .werft/aks-installer-tests.yaml -a skipTests=true
werft run github -j .werft/k3s-installer-tests.yaml -a skipTests=true
werft run github -j .werft/eks-installer-tests.yaml -a skipTests=true

There's a possibility that these may fail due to these tests being fairly new, but this should give us an idea if this change creates any regressions.

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-pd-fix-blobserve.3 because the annotations in the pull request description changed
(with .werft/ from main)

@iQQBot
Copy link
Contributor Author

iQQBot commented Sep 5, 2022

I'm broadly ok with this change, but I don't know the implications of changing the network policies. My recollection from when we did this is that it was only implemented in GKE (not EKS, AKS or k3s).

The role of network policy is 2 parts

One is to restrict access to certain non-public services, such as ws-manager, which we don't want some pod like the workspace pod to be able to access, which may pose some security issues

The other role is to unrestrict access and prevent other network policies from acting on this pod

For blobserve, it is a public service, there is no need to restrict access to it, so in this PR, we remove all access restrictions

Copy link
Contributor

@corneliusludmann corneliusludmann left a comment

Choose a reason for hiding this comment

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

I tested the PR and it seems to fix the issue. Thank you very much @iQQBot for investigating this! 🚀

And sorry that it took so long on my end.

/unhold

@roboquat roboquat merged commit e6e40d4 into main Sep 9, 2022
@roboquat roboquat deleted the pd/fix-blobserve branch September 9, 2022 09:50
@roboquat roboquat added the deployed: IDE IDE change is running in production label Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: IDE IDE change is running in production release-note-none size/L team: delivery Issue belongs to the self-hosted team team: IDE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Workspaces stuck in Pulling state
5 participants