Skip to content

Commit

Permalink
address PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@sagor999
sagor999 committed May 19, 2022
1 parent ea130aa commit f049320
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 59 deletions.
4 changes: 2 additions & 2 deletions components/ws-daemon/pkg/internal/session/workspace.go
View file
Expand Up @@ -280,9 +280,9 @@ func (s *Workspace) UpdateGitSafeDirectory(ctx context.Context) (err error) {
}

// UpdateGitStatus attempts to update the LastGitStatus from the workspace's local working copy.
func (s *Workspace) UpdateGitStatus(ctx context.Context, persistent_volume_claim bool) (res *csapi.GitStatus, err error) {
func (s *Workspace) UpdateGitStatus(ctx context.Context, persistentVolumeClaim bool) (res *csapi.GitStatus, err error) {
var loc string
if persistent_volume_claim {
if persistentVolumeClaim {
loc = filepath.Join(s.ServiceLocDaemon, "prestophookdata")
stat, err := git.GitStatusFromFiles(ctx, loc)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion components/ws-manager/cmd/run.go
View file
Expand Up @@ -32,7 +32,7 @@ import (
imgbldr "github.com/gitpod-io/gitpod/image-builder/api"
"github.com/gitpod-io/gitpod/ws-manager/pkg/manager"
"github.com/gitpod-io/gitpod/ws-manager/pkg/proxy"
volumesnapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1"
volumesnapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1"
)

// serveCmd represents the serve command
Expand Down
3 changes: 1 addition & 2 deletions components/ws-manager/go.mod
View file
Expand Up @@ -20,6 +20,7 @@ require (
github.com/google/uuid v1.3.0
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0
github.com/imdario/mergo v0.3.12
github.com/kubernetes-csi/external-snapshotter/client/v4 v4.2.0
github.com/opentracing/opentracing-go v1.2.0
github.com/prometheus/client_golang v1.12.1
github.com/sirupsen/logrus v1.8.1
Expand Down Expand Up @@ -73,8 +74,6 @@ require (
github.com/inconshreveable/mousetrap v1.0.0 // indirect
github.com/json-iterator/go v1.1.12 // indirect
github.com/klauspost/cpuid v1.3.1 // indirect
github.com/klauspost/pgzip v1.2.5 // indirect
github.com/kubernetes-csi/external-snapshotter/client/v6 v6.0.0-rc4 // indirect
github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect
github.com/minio/md5-simd v1.1.0 // indirect
github.com/minio/minio-go/v7 v7.0.11 // indirect
Expand Down
7 changes: 3 additions & 4 deletions components/ws-manager/go.sum
View file
Expand Up @@ -406,10 +406,8 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
github.com/linuxkit/virtsock v0.0.0-20201010232012-f8cee7dfc7a3/go.mod h1:3r6x7q95whyfWQpmGZTu3gk3v2YkMi05HEzl7Tf7YEo=
github.com/kubernetes-csi/external-snapshotter/client/v6 v6.0.0-rc4 h1:uspy64y8fTrwchSk4dKx/CAndt0y2V70BFM2C9/jTIE=
github.com/kubernetes-csi/external-snapshotter/client/v6 v6.0.0-rc4/go.mod h1:tnHiLn3P10N95fjn7O40QH5ovN0EFGAxqdTpUMrX6bU=
github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
github.com/kubernetes-csi/external-snapshotter/client/v4 v4.2.0 h1:nHHjmvjitIiyPlUHk/ofpgvBcNcawJLtf4PYHORLjAA=
github.com/kubernetes-csi/external-snapshotter/client/v4 v4.2.0/go.mod h1:YBCo4DoEeDndqvAn6eeu0vWM7QdXmHEeI9cFWplmBys=
github.com/magiconair/properties v1.8.1/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
github.com/magiconair/properties v1.8.5 h1:b6kJs+EmPFMYGkow9GiUyCyOvIwYetYJ3fSaWak/Gls=
github.com/magiconair/properties v1.8.5/go.mod h1:y3VJvCyxH9uVvJTWEGAELF3aiYNyPKd5NZ3oSwXrF60=
Expand Down Expand Up @@ -855,6 +853,7 @@ golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20200416051211-89c76fbcd5d1/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20210220033141-f8bda1e9f3ba/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20210723032227-1f47c861a9ac/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20220411224347-583f2d630306 h1:+gHMid33q6pen7kv9xvT+JRinntgeXO2AeZVd0AWD3w=
Expand Down
13 changes: 8 additions & 5 deletions components/ws-manager/pkg/manager/create.go
View file
Expand Up @@ -33,6 +33,7 @@ import (
regapi "github.com/gitpod-io/gitpod/registry-facade/api"
"github.com/gitpod-io/gitpod/ws-manager/api"
config "github.com/gitpod-io/gitpod/ws-manager/api/config"
volumesnapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1"
)

// Protobuf structures often require pointer to boolean values (as that's Go's best means of expression optionallity).
Expand Down Expand Up @@ -255,8 +256,8 @@ func (m *Manager) createPVCForWorkspacePod(startContext *startWorkspaceContext)
},
}

if startContext.VolumeSnapshot.VolumeSnapshotName != "" {
snapshotApiGroup := "snapshot.storage.k8s.io"
if startContext.VolumeSnapshot != nil {
snapshotApiGroup := volumesnapshotv1.GroupName
PVC.Spec.DataSource = &corev1.TypedLocalObjectReference{
APIGroup: &snapshotApiGroup,
Kind: "VolumeSnapshot",
Expand Down Expand Up @@ -903,10 +904,12 @@ func (m *Manager) newStartWorkspaceContext(ctx context.Context, req *api.StartWo
workspaceClassLabel: clsName,
}

var volumeSnapshot workspaceVolumeSnapshotStatus
var volumeSnapshot *workspaceVolumeSnapshotStatus
if req.Spec.VolumeSnapshot != nil {
volumeSnapshot.VolumeSnapshotName = req.Spec.VolumeSnapshot.VolumeSnapshotName
volumeSnapshot.VolumeSnapshotHandle = req.Spec.VolumeSnapshot.VolumeSnapshotHandle
volumeSnapshot = &workspaceVolumeSnapshotStatus{
VolumeSnapshotName: req.Spec.VolumeSnapshot.VolumeSnapshotName,
VolumeSnapshotHandle: req.Spec.VolumeSnapshot.VolumeSnapshotHandle,
}
}

return &startWorkspaceContext{
Expand Down
22 changes: 11 additions & 11 deletions components/ws-manager/pkg/manager/manager.go
View file
Expand Up @@ -68,17 +68,17 @@ type Manager struct {
}

type startWorkspaceContext struct {
Request *api.StartWorkspaceRequest `json:"request"`
Labels map[string]string `json:"labels"`
CLIAPIKey string `json:"cliApiKey"`
OwnerToken string `json:"ownerToken"`
IDEPort int32 `json:"idePort"`
SupervisorPort int32 `json:"supervisorPort"`
WorkspaceURL string `json:"workspaceURL"`
TraceID string `json:"traceID"`
Headless bool `json:"headless"`
Class *config.WorkspaceClass `json:"class"`
VolumeSnapshot workspaceVolumeSnapshotStatus `json:"volumeSnapshot"`
Request *api.StartWorkspaceRequest `json:"request"`
Labels map[string]string `json:"labels"`
CLIAPIKey string `json:"cliApiKey"`
OwnerToken string `json:"ownerToken"`
IDEPort int32 `json:"idePort"`
SupervisorPort int32 `json:"supervisorPort"`
WorkspaceURL string `json:"workspaceURL"`
TraceID string `json:"traceID"`
Headless bool `json:"headless"`
Class *config.WorkspaceClass `json:"class"`
VolumeSnapshot *workspaceVolumeSnapshotStatus `json:"volumeSnapshot"`
}

func (swctx *startWorkspaceContext) ContainerConfiguration() config.ContainerConfiguration {
Expand Down
66 changes: 32 additions & 34 deletions components/ws-manager/pkg/manager/monitor.go
View file
Expand Up @@ -39,7 +39,7 @@ import (
"github.com/gitpod-io/gitpod/ws-manager/api"
"github.com/gitpod-io/gitpod/ws-manager/pkg/manager/internal/workpool"

volumesnapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1"
volumesnapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1"
)

const (
Expand Down Expand Up @@ -870,13 +870,13 @@ func (m *Monitor) finalizeWorkspaceContent(ctx context.Context, wso *workspaceOb
deletedPVC bool
pvcFeatureEnabled bool
markVolumeSnapshotAnnotation bool
pvcVolumeSnapshotName string
// volume snapshot name is 1:1 mapped to workspace id
pvcVolumeSnapshotName string = workspaceID
pvcVolumeSnapshotContentName string
pvcVolumeSnapshotClassName string
)
if wso.Pod != nil {
_, pvcFeatureEnabled = wso.Pod.Labels[pvcWorkspaceFeatureAnnotation]
pvcVolumeSnapshotName = workspaceID
wsClassName := ""
if _, ok := wso.Pod.Labels[workspaceClassLabel]; ok {
wsClassName = wso.Pod.Labels[workspaceClassLabel]
Expand Down Expand Up @@ -929,16 +929,14 @@ func (m *Monitor) finalizeWorkspaceContent(ctx context.Context, wso *workspaceOb
}

if pvcFeatureEnabled {
// pvc was created with the name of the pod. see createDefiniteWorkspacePod()
pvcName := wso.Pod.Name
if !createdVolumeSnapshot {
// create snapshot object out of PVC
volumeSnapshot := &volumesnapshotv1.VolumeSnapshot{
ObjectMeta: metav1.ObjectMeta{
Name: pvcVolumeSnapshotName,
Namespace: m.manager.Config.Namespace,
Labels: map[string]string{
"workspaceID": workspaceID,
},
},
Spec: volumesnapshotv1.VolumeSnapshotSpec{
Source: volumesnapshotv1.VolumeSnapshotSource{
Expand All @@ -949,7 +947,7 @@ func (m *Monitor) finalizeWorkspaceContent(ctx context.Context, wso *workspaceOb
}

err = m.manager.Clientset.Create(ctx, volumeSnapshot)
if err != nil {
if err != nil && !k8serr.IsAlreadyExists(err) {
err = xerrors.Errorf("cannot create volumesnapshot: %v", err)
return true, nil, err
}
Expand All @@ -963,6 +961,7 @@ func (m *Monitor) finalizeWorkspaceContent(ctx context.Context, wso *workspaceOb
Jitter: 0.1,
Cap: 10 * time.Minute,
}
log = log.WithField("VolumeSnapshot.Name", pvcVolumeSnapshotName)
err = wait.ExponentialBackoff(backoff, func() (bool, error) {
var volumeSnapshot volumesnapshotv1.VolumeSnapshot
err := m.manager.Clientset.Get(ctx, types.NamespacedName{Namespace: m.manager.Config.Namespace, Name: pvcVolumeSnapshotName}, &volumeSnapshot)
Expand All @@ -971,37 +970,38 @@ func (m *Monitor) finalizeWorkspaceContent(ctx context.Context, wso *workspaceOb
// volumesnapshot doesn't exist yet, retry again
return false, nil
}
log.WithError(err).WithField("VolumeSnapshot.Name", pvcVolumeSnapshotName).Error("was unable to get volume snapshot")
log.WithError(err).Error("was unable to get volume snapshot")
return false, err
}
if volumeSnapshot.Status != nil {
if volumeSnapshot.Status.ReadyToUse != nil && *(volumeSnapshot.Status.ReadyToUse) {
if volumeSnapshot.Status.ReadyToUse != nil && *(volumeSnapshot.Status.ReadyToUse) && volumeSnapshot.Status.BoundVolumeSnapshotContentName != nil {
pvcVolumeSnapshotContentName = *volumeSnapshot.Status.BoundVolumeSnapshotContentName
return true, nil
}
if volumeSnapshot.Status.Error != nil {
if volumeSnapshot.Status.Error.Message != nil {
err = xerrors.Errorf("error during volume snapshot creation: %s", *volumeSnapshot.Status.Error.Message)
log.WithError(err).WithField("VolumeSnapshot.Name", pvcVolumeSnapshotName).Error("unable to create volume snapshot")
log.WithError(err).Error("unable to create volume snapshot")
return false, err
}
log.WithField("VolumeSnapshot.Name", pvcVolumeSnapshotName).Error("unknown error during volume snapshot creation")
log.Error("unknown error during volume snapshot creation")
return false, xerrors.Errorf("unknown error during volume snapshot creation")
}
}
return false, nil
})
if err != nil {
log.WithError(err).WithField("VolumeSnapshot.Name", pvcVolumeSnapshotName).Errorf("failed while waiting for volume snapshot to get ready")
log.WithError(err).Errorf("failed while waiting for volume snapshot to get ready")
return true, nil, err
}
readyVolumeSnapshot = true
}
if readyVolumeSnapshot && !markVolumeSnapshotAnnotation {
log = log.WithField("VolumeSnapshotContent.Name", pvcVolumeSnapshotContentName)
var volumeSnapshotContent volumesnapshotv1.VolumeSnapshotContent
err := m.manager.Clientset.Get(ctx, types.NamespacedName{Namespace: "", Name: pvcVolumeSnapshotContentName}, &volumeSnapshotContent)
if err != nil {
log.WithError(err).WithField("VolumeSnapshotContent.Name", pvcVolumeSnapshotContentName).Error("was unable to get volume snapshot content")
log.WithError(err).Error("was unable to get volume snapshot content")
return true, nil, err
}

Expand Down Expand Up @@ -1038,38 +1038,36 @@ func (m *Monitor) finalizeWorkspaceContent(ctx context.Context, wso *workspaceOb
},
},
)
if pvcErr != nil {
if pvcErr != nil && !k8serr.IsNotFound(pvcErr) {
log.WithError(pvcErr).Errorf("failed to delete pvc `%s`", pvcName)
}
deletedPVC = true
}
} else {
if doSnapshot {
// if this is a prebuild take a snapshot and mark the workspace
var res *wsdaemon.TakeSnapshotResponse
res, err = snc.TakeSnapshot(ctx, &wsdaemon.TakeSnapshotRequest{Id: workspaceID})
} else if doSnapshot {
// if this is a prebuild take a snapshot and mark the workspace
var res *wsdaemon.TakeSnapshotResponse
res, err = snc.TakeSnapshot(ctx, &wsdaemon.TakeSnapshotRequest{Id: workspaceID})
if err != nil {
tracing.LogError(span, err)
log.WithError(err).Warn("cannot take snapshot")
err = xerrors.Errorf("cannot take snapshot: %v", err)
err = m.manager.markWorkspace(ctx, workspaceID, addMark(workspaceExplicitFailAnnotation, err.Error()))
if err != nil {
log.WithError(err).Warn("was unable to mark workspace as failed")
}
}

if res != nil {
err = m.manager.markWorkspace(context.Background(), workspaceID, addMark(workspaceSnapshotAnnotation, res.Url))
if err != nil {
tracing.LogError(span, err)
log.WithError(err).Warn("cannot take snapshot")
err = xerrors.Errorf("cannot take snapshot: %v", err)
log.WithError(err).Warn("cannot mark headless workspace with snapshot - that's one prebuild lost")
err = xerrors.Errorf("cannot remember snapshot: %v", err)
err = m.manager.markWorkspace(ctx, workspaceID, addMark(workspaceExplicitFailAnnotation, err.Error()))
if err != nil {
log.WithError(err).Warn("was unable to mark workspace as failed")
}
}

if res != nil {
err = m.manager.markWorkspace(context.Background(), workspaceID, addMark(workspaceSnapshotAnnotation, res.Url))
if err != nil {
tracing.LogError(span, err)
log.WithError(err).Warn("cannot mark headless workspace with snapshot - that's one prebuild lost")
err = xerrors.Errorf("cannot remember snapshot: %v", err)
err = m.manager.markWorkspace(ctx, workspaceID, addMark(workspaceExplicitFailAnnotation, err.Error()))
if err != nil {
log.WithError(err).Warn("was unable to mark workspace as failed")
}
}
}
}
}

Expand Down

0 comments on commit f049320

Please sign in to comment.