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

feat: wip max in-flight pulls #81

Closed
wants to merge 18 commits into from
Closed

feat: wip max in-flight pulls #81

wants to merge 18 commits into from

Conversation

vadasambar
Copy link
Contributor

Fixes #77

@vadasambar vadasambar changed the title feat: max in-flight pulls [DON'T REVIEW] feat: wip max in-flight pulls Dec 11, 2023
@vadasambar
Copy link
Contributor Author

For now, pulling changes from #83 for testing this PR.

@vadasambar
Copy link
Contributor Author

Ran tests:

$ cd cmd/plugin
$ go test -run TestNodePublishVolumeAsyncInFlightPulls

Test result

I1219 12:01:28.785605   28062 server.go:108] Listening for connections on address: &net.UnixAddr{Name:"//tmp/csi.sock", Net:"unix"}
I1219 12:01:30.789678   28062 node_server.go:62] mount request: volume_id:"docker.io/library/redis:latest" target_path:"test-path1" volume_capability:<access_mode:<mode:SINGLE_NODE_READER_ONLY > > volume_context:<key:"pullAlways" value:"true" > 
I1219 12:01:30.795363   28062 pullexecutor.go:123] pull image "docker.io/library/redis:latest" 
I1219 12:01:30.887674   28062 node_server.go:62] mount request: volume_id:"docker.io/library/ubuntu:latest" target_path:"test-path2" volume_capability:<access_mode:<mode:SINGLE_NODE_READER_ONLY > > volume_context:<key:"pullAlways" value:"true" > 
E1219 12:01:35.788257   28062 node_server_test.go:266] context deadline exceeded; retrying: rpc error: code = DeadlineExceeded desc = context deadline exceeded
I1219 12:01:35.795929   28062 pullexecutor.go:123] pull image "docker.io/library/ubuntu:latest" 
E1219 12:01:35.888196   28062 node_server_test.go:281] context deadline exceeded; retrying: rpc error: code = DeadlineExceeded desc = context deadline exceeded
I1219 12:01:35.888497   28062 node_server.go:62] mount request: volume_id:"docker.io/library/redis:latest" target_path:"test-path1" volume_capability:<access_mode:<mode:SINGLE_NODE_READER_ONLY > > volume_context:<key:"pullAlways" value:"true" > 
I1219 12:01:35.989549   28062 node_server.go:62] mount request: volume_id:"docker.io/library/ubuntu:latest" target_path:"test-path2" volume_capability:<access_mode:<mode:SINGLE_NODE_READER_ONLY > > volume_context:<key:"pullAlways" value:"true" > 
I1219 12:01:45.896044   28062 node_server_test.go:235] server was stopped
PASS
ok  	github.com/warm-metal/csi-driver-image/cmd/plugin	17.135s

@vadasambar vadasambar changed the title [DON'T REVIEW] feat: wip max in-flight pulls feat: wip max in-flight pulls Dec 19, 2023
- makes the tests easier to run
- edit docs to run node server with containerd image

refactor: remove commented out code

chore: switch over from criapi `v1alpha2` -> `v1`
test: use mock puller and mounter in the node server tests
- makes the tests easier to run
- edit docs to run node server with containerd image

feat: first implementation of the tokens logic

test: add test for in-flight pulls

refactor: use string instead of docker named ref
- works for now but we might face problems in the future if the docker named ref starts using pointer fields

chore: fix syntax error after rebase

refactor: use named ref string instead of named ref struct directly
- because named ref struct might not always satisfy equality (it does no because the struct is shallow)

feat: expose `--max-in-flight-pulls` as a flag
- so that users can set the value through flag or helm chart

docs: add log message for when the image is downloaded
- helps with testing max in-flight image pulls
- because of syntax errors
- makes it easier to test `--max-in-flight-pulls`
- expose the flags via helm values
- add integration test to test `--max-in-flight-pulls` flag
- move metrics integration tests to generic integration tests
- use the correct namespace (`kube-system` instead of `default`)
@@ -78,7 +78,7 @@ func (n NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublishV
return
}

// to trigger CI
// to trigger CI again
Copy link
Contributor

Choose a reason for hiding this comment

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

@vadasambar You might consider using allow-empty. You'll not need to remove any unnecessary lines later.

git commit --allow-empty --message "🤖 Trigger CI run"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is helpful. Thank you Mriyam.

Copy link

github-actions bot commented Mar 5, 2024

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Mar 5, 2024
Copy link

Closing this PR after a prolonged period of inactivity. Please create a new PR if the changes of the PR are still relevant.

@github-actions github-actions bot closed this Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support in-flight requests for warm metal driver
2 participants