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

GCP object storage bugfixes #6636

Merged
merged 3 commits into from
Nov 12, 2021
Merged

GCP object storage bugfixes #6636

merged 3 commits into from
Nov 12, 2021

Conversation

mrsimonemms
Copy link
Contributor

@mrsimonemms mrsimonemms commented Nov 10, 2021

Description

Depends on #6621 being merged to main first

  • remove minio.<domain> from proxy if using GCP cloud storage
  • removed duplicate storageConfiguration function in common module
  • update the config generator to make metadata.region to be local. Enforce this value in the storage configuration if using Minio - I've found that if the config value is not local, it causes connectivity issues.

Related Issue(s)

Fixes #

How to test

Create a service account with "roles/storage.admin and roles/storage.objectAdmin roles and create a secret:

kubectl create secret generic remote-storage-gcloud --from-file=service-account.json=./gs-credentials.json

Deploy using config:

objectStorage:
  cloudStorage:
    serviceAccount:
      kind: secret
      name: remote-storage-gcloud
    project: sje-self-hosted-playground

Ensure that incluster config is fine by running objectStorage.inCluster: true

Release Notes

GCP object storage bugfixes 

Documentation

@codecov
Copy link

codecov bot commented Nov 10, 2021

Codecov Report

Merging #6636 (20e0632) into main (a7166da) will increase coverage by 4.34%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6636      +/-   ##
==========================================
+ Coverage   32.74%   37.08%   +4.34%     
==========================================
  Files         122       85      -37     
  Lines       22193    16656    -5537     
==========================================
- Hits         7266     6177    -1089     
+ Misses      14309     9997    -4312     
+ Partials      618      482     -136     
Flag Coverage Δ
components-blobserve-app 30.43% <ø> (ø)
components-content-service-app ?
components-content-service-lib ?
components-ee-agent-smith-app 40.05% <ø> (-0.28%) ⬇️
components-ee-agent-smith-lib 40.32% <ø> (ø)
components-ee-kedge-app ?
components-ee-ws-scheduler-app 63.92% <ø> (ø)
components-image-builder-app ?
components-image-builder-mk3-app 35.03% <ø> (ø)
components-local-app-app-darwin-amd64 ?
components-local-app-app-darwin-arm64 ?
components-local-app-app-linux-amd64 ?
components-local-app-app-linux-arm64 ?
components-local-app-app-windows-386 ?
components-local-app-app-windows-amd64 ?
components-local-app-app-windows-arm64 ?
components-registry-facade-app 11.54% <ø> (ø)
components-service-waiter-app ?
components-supervisor-app 37.02% <ø> (-0.35%) ⬇️
components-workspacekit-app 7.05% <ø> (ø)
components-ws-daemon-app ?
components-ws-daemon-lib ?
components-ws-manager-app 39.64% <ø> (ø)
components-ws-proxy-app 69.80% <ø> (ø)
components-ws-proxy-lib 69.80% <ø> (ø)
dev-poolkeeper-app ?
installer-raw-app 6.13% <0.00%> (+0.13%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
installer/pkg/common/objects.go 0.00% <ø> (ø)
installer/pkg/common/storage.go 0.00% <0.00%> (ø)
installer/pkg/components/ws-manager/configmap.go 29.71% <0.00%> (+0.66%) ⬆️
components/supervisor/pkg/ports/ports.go 59.90% <0.00%> (-2.30%) ⬇️
components/image-builder/pkg/resolve/resolve.go 32.65% <0.00%> (-1.62%) ⬇️
components/supervisor/pkg/terminal/terminal.go 63.69% <0.00%> (-0.62%) ⬇️
components/ws-daemon/pkg/content/tar.go
components/ee/kedge/pkg/kedge/collector.go
components/ws-daemon/pkg/content/hooks.go
components/content-service/pkg/git/git.go
... and 33 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7166da...20e0632. Read the comment docs.

@mrsimonemms mrsimonemms force-pushed the sje/gke-obj-storage branch 2 times, most recently from 973e50c to 5dcaeaa Compare November 10, 2021 13:03
@roboquat roboquat added size/XL and removed size/L labels Nov 10, 2021
@mrsimonemms mrsimonemms changed the title Sje/gke obj storage GCP object storage bugfixes Nov 10, 2021
@@ -23,6 +23,68 @@ func deployment(ctx *common.RenderContext) ([]runtime.Object, error) {
return nil, err
}

podSpec := corev1.PodSpec{
Copy link
Contributor Author

@mrsimonemms mrsimonemms Nov 10, 2021

Choose a reason for hiding this comment

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

Although this looks a big change, it's "just" copy/pasta so can put the AddStorageMounts in

@mrsimonemms mrsimonemms marked this pull request as ready for review November 10, 2021 13:43
@mrsimonemms
Copy link
Contributor Author

/assign @csweichel

Copy link
Contributor

@csweichel csweichel left a comment

Choose a reason for hiding this comment

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

will take another look once the external registry PR is merged

installer/pkg/common/storage.go Outdated Show resolved Hide resolved
@mrsimonemms mrsimonemms force-pushed the sje/gke-obj-storage branch 2 times, most recently from b0d847c to 2ead0b1 Compare November 11, 2021 08:16
@csweichel
Copy link
Contributor

/lgtm

@roboquat roboquat added the lgtm label Nov 12, 2021
@roboquat
Copy link
Contributor

LGTM label has been added.

Git tree hash: aa6b7dcbe53d632525f3e3c5c2c7dd032179ec84

@roboquat
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: csweichel

Associated issue: #6621

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@roboquat roboquat merged commit 43f041d into main Nov 12, 2021
@roboquat roboquat deleted the sje/gke-obj-storage branch November 12, 2021 10:25
@roboquat roboquat added the deployed: webapp Meta team change is running in production label Nov 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved deployed: webapp Meta team change is running in production release-note size/L team: webapp Issue belongs to the WebApp team team: workspace Issue belongs to the Workspace team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants