align go repository - #174
Conversation
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
94c04dd to
0ca7552
Compare
0ca7552 to
569c68c
Compare
| // Fetch source volume to determine storage type | ||
| sourceVolume, err := d.cloudscaleClient.Volumes.Get(ctx, snapshot.SourceVolume.UUID) | ||
| if err != nil { | ||
| return nil, status.Errorf(codes.Internal, "failed to get source volume for snapshot: %v", err) |
There was a problem hiding this comment.
this is the only (?) line that does not do the errors.AsType[*cloudscale.ErrorResponse](err) dance?
| } | ||
|
|
||
| if err := drv.Run(); err != nil { | ||
| ctx, cancel := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) |
There was a problem hiding this comment.
I think this breaks the shutdown, you can either deploy it to a cluster and observe the logs, or test with this:
diff --git a/driver/driver.go b/driver/driver.go
index ddbd5e4..6709dc0 100644
--- a/driver/driver.go
+++ b/driver/driver.go
@@ -84,16 +84,14 @@ func NewDriver(ep, token, urlstr string, logLevel logrus.Level) (*Driver, error)
})
oauthClient := oauth2.NewClient(context.Background(), tokenSource)
- metadataClient := cloudscale.NewMetadataClient(nil)
- metadata, err := metadataClient.GetMetadata(context.Background())
- if err != nil {
- return nil, fmt.Errorf("couldn't get metadata: %s", err)
+ if os.Getenv("CSI_LOCAL_DEMO") != "1" {
+ panic("set CSI_LOCAL_DEMO=1 to run this demo build")
}
-
- // We don't have any other information than the availability zone. Just use
- // it as the zone for now.
- zone := metadata.AvailabilityZone
- serverID := metadata.Meta.CloudscaleUUID
+ zone := "demo-zone"
+ serverID := "demo-server-id"
cloudscaleClient := cloudscale.NewClient(oauthClient)
baseURL, err := url.Parse(urlstr)Then:
go build -o /tmp/demo-plugin ./cmd/cloudscale-csi-plugin
CSI_LOCAL_DEMO=1 /tmp/demo-plugin --endpoint unix:///tmp/demo.sock --token dummy-token
now: ctrl-c cannot exit the process.
There was a problem hiding this comment.
oh, nice catch 🤦 yeah, needs some more plumping to make it work
| .git | ||
| .gitignore | ||
| *.md | ||
| .github/ | ||
| /charts/ | ||
| deploy/ | ||
| examples/ | ||
| helpers/ | ||
| scripts/ | ||
| test/ | ||
| go.mod | ||
| go.sum | ||
| Dockerfile | ||
| Makefile | ||
| VERSION |
There was a problem hiding this comment.
AFAICT this file serves no purpose looking at the Dockerfile: it only ever adds two very specific paths.
or do I miss something?
| go-version-file: go.mod | ||
|
|
||
| - name: Check Go modules | ||
| run: go mod tidy |
There was a problem hiding this comment.
this does never fail: it just modifies go.mod and exits with 0.
compare with old version from Makefile:
.PHONY: check-unused
check-unused:
@git diff --exit-code -- go.sum go.mod || ( echo "there are uncommitted changes to the go.mod/go.sum -- please run 'go mod tidy' and commit the changes first"; exit 1 )
| if err := luksClose(ctx, mappingName, log); err != nil { | ||
| return err | ||
| } | ||
| } |
There was a problem hiding this comment.
Can you please consider this case:
isLuksMapping, mappingName, err := isLuksMapping(ctx, info.Source) // <- returns context.Canceled
if err != nil {
return err // we exit here
}
if isLuksMapping {
if err := luksClose(ctx, mappingName, log); err != nil { // not calledand then we never execute cryptsetup close.. I think it necessitates a WithoutCancel.
3 participants
No description provided.