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

feature: extend cri apis for remove volume #2124

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
986 changes: 670 additions & 316 deletions cri/apis/v1alpha2/api.pb.go

Large diffs are not rendered by default.

18 changes: 17 additions & 1 deletion cri/apis/v1alpha2/api.proto
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,12 @@ service ImageService {
rpc ImageFsInfo(ImageFsInfoRequest) returns (ImageFsInfoResponse) {}
}

// VolumeService defines the public APIs for managing volumes.
service VolumeService {
// RemoveVolume removes the volume.
rpc RemoveVolume(RemoveVolumeRequest) returns (RemoveVolumeResponse) {}
}

message VersionRequest {
// Version of the kubelet runtime API.
string version = 1;
Expand Down Expand Up @@ -181,6 +187,8 @@ message Mount {
bool selinux_relabel = 4;
// Requested propagation mode.
MountPropagation propagation = 5;
// Name of volume
Copy link
Collaborator

Choose a reason for hiding this comment

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

TODO:
classify the anonymous volume or not.

string name = 100;
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need add more param in Mount?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's enough for the moment. :)

}

// A NamespaceMode describes the intended namespace configuration for each
Expand Down Expand Up @@ -1282,4 +1290,12 @@ message ReopenContainerLogRequest {
}

message ReopenContainerLogResponse{
}
}

message RemoveVolumeRequest {
// Name of the volume to remove
string volume_name = 1;
}

message RemoveVolumeResponse {
}
8 changes: 4 additions & 4 deletions cri/criservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
)

// RunCriService start cri service if pouchd is specified with --enable-cri.
func RunCriService(daemonconfig *config.Config, containerMgr mgr.ContainerMgr, imageMgr mgr.ImageMgr, stopCh chan error, readyCh chan bool) {
func RunCriService(daemonconfig *config.Config, containerMgr mgr.ContainerMgr, imageMgr mgr.ImageMgr, volumeMgr mgr.VolumeMgr, stopCh chan error, readyCh chan bool) {
var err error

defer func() {
Expand All @@ -30,7 +30,7 @@ func RunCriService(daemonconfig *config.Config, containerMgr mgr.ContainerMgr, i
case "v1alpha1":
err = runv1alpha1(daemonconfig, containerMgr, imageMgr, readyCh)
case "v1alpha2":
err = runv1alpha2(daemonconfig, containerMgr, imageMgr, readyCh)
err = runv1alpha2(daemonconfig, containerMgr, imageMgr, volumeMgr, readyCh)
default:
readyCh <- false
err = fmt.Errorf("invalid CRI version,failed to start CRI service")
Expand Down Expand Up @@ -79,9 +79,9 @@ func runv1alpha1(daemonconfig *config.Config, containerMgr mgr.ContainerMgr, ima
}

// Start CRI service with CRI version: v1alpha2
func runv1alpha2(daemonconfig *config.Config, containerMgr mgr.ContainerMgr, imageMgr mgr.ImageMgr, readyCh chan bool) error {
func runv1alpha2(daemonconfig *config.Config, containerMgr mgr.ContainerMgr, imageMgr mgr.ImageMgr, volumeMgr mgr.VolumeMgr, readyCh chan bool) error {
logrus.Infof("Start CRI service with CRI version: v1alpha2")
criMgr, err := criv1alpha2.NewCriManager(daemonconfig, containerMgr, imageMgr)
criMgr, err := criv1alpha2.NewCriManager(daemonconfig, containerMgr, imageMgr, volumeMgr)
if err != nil {
readyCh <- false
return fmt.Errorf("failed to get CriManager with error: %v", err)
Expand Down
17 changes: 16 additions & 1 deletion cri/v1alpha2/cri.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ type CriMgr interface {
// ImageServiceServer is interface of CRI image service.
runtime.ImageServiceServer

// VolumeServiceServer is interface of CRI volume service.
runtime.VolumeServiceServer

// StreamServerStart starts the stream server of CRI.
StreamServerStart() error
}
Expand All @@ -99,6 +102,7 @@ type CriMgr interface {
type CriManager struct {
ContainerMgr mgr.ContainerMgr
ImageMgr mgr.ImageMgr
VolumeMgr mgr.VolumeMgr
CniMgr cni.CniMgr

// StreamServer is the stream server of CRI serves container streaming request.
Expand All @@ -121,7 +125,7 @@ type CriManager struct {
}

// NewCriManager creates a brand new cri manager.
func NewCriManager(config *config.Config, ctrMgr mgr.ContainerMgr, imgMgr mgr.ImageMgr) (CriMgr, error) {
func NewCriManager(config *config.Config, ctrMgr mgr.ContainerMgr, imgMgr mgr.ImageMgr, volumeMgr mgr.VolumeMgr) (CriMgr, error) {
streamServer, err := newStreamServer(ctrMgr, streamServerAddress, streamServerPort)
if err != nil {
return nil, fmt.Errorf("failed to create stream server for cri manager: %v", err)
Expand All @@ -130,6 +134,7 @@ func NewCriManager(config *config.Config, ctrMgr mgr.ContainerMgr, imgMgr mgr.Im
c := &CriManager{
ContainerMgr: ctrMgr,
ImageMgr: imgMgr,
VolumeMgr: volumeMgr,
CniMgr: cni.NewCniManager(&config.CriConfig),
StreamServer: streamServer,
SandboxBaseDir: path.Join(config.HomeDir, "sandboxes"),
Expand Down Expand Up @@ -671,6 +676,7 @@ func (c *CriManager) ContainerStatus(ctx context.Context, r *runtime.ContainerSt
HostPath: m.Source,
ContainerPath: m.Destination,
Readonly: !m.RW,
Name: m.Name,
// Note: can't set SeLinuxRelabel.
})
}
Expand Down Expand Up @@ -1104,3 +1110,12 @@ func (c *CriManager) ImageFsInfo(ctx context.Context, r *runtime.ImageFsInfoRequ
},
}, nil
}

// RemoveVolume removes the volume.
func (c *CriManager) RemoveVolume(ctx context.Context, r *runtime.RemoveVolumeRequest) (*runtime.RemoveVolumeResponse, error) {
volumeName := r.GetVolumeName()
if err := c.VolumeMgr.Remove(ctx, volumeName); err != nil {
return nil, err
}
return &runtime.RemoveVolumeResponse{}, nil
}
13 changes: 13 additions & 0 deletions cri/v1alpha2/cri_wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -400,3 +400,16 @@ func (c *CriWrapper) ImageFsInfo(ctx context.Context, r *runtime.ImageFsInfoRequ
}()
return c.CriManager.ImageFsInfo(ctx, r)
}

// RemoveVolume removes the volume.
func (c *CriWrapper) RemoveVolume(ctx context.Context, r *runtime.RemoveVolumeRequest) (res *runtime.RemoveVolumeResponse, err error) {
logrus.Infof("RemoveVolume %q", r.GetVolumeName())
defer func() {
if err != nil {
logrus.Errorf("RemoveVolume %q failed, error: %v", r.GetVolumeName(), err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

failed to do something

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll submit a single PR to format logs.

} else {
logrus.Infof("RemoveVolume %q returns successfully", r.GetVolumeName())
}
}()
return c.CriManager.RemoveVolume(ctx, r)
}
1 change: 1 addition & 0 deletions cri/v1alpha2/service/cri.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ func NewService(cfg *config.Config, criMgr cri.CriMgr) (*Service, error) {

runtime.RegisterRuntimeServiceServer(s.server, s.criMgr)
runtime.RegisterImageServiceServer(s.server, s.criMgr)
runtime.RegisterVolumeServiceServer(s.server, s.criMgr)

return s, nil
}
Expand Down
2 changes: 1 addition & 1 deletion daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ func (d *Daemon) Run() error {
}()

criStopCh := make(chan error)
go criservice.RunCriService(d.config, d.containerMgr, d.imageMgr, criStopCh, criReadyCh)
go criservice.RunCriService(d.config, d.containerMgr, d.imageMgr, d.volumeMgr, criStopCh, criReadyCh)

httpReady := <-httpReadyCh
criReady := <-criReadyCh
Expand Down
1 change: 0 additions & 1 deletion hack/protoc.sh
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ generateproto(){
}

main(){
API_ROOT="${DIR}/cri/apis/v1alpha1" YEAR_TIME=" $(date '+%Y')" generateproto
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why delete it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now we don't need to extend the API of CRI v1alpha1. :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we do not the extensive CRI advanced feature, should we find somewhere to record this change? @starnop

Copy link
Collaborator

Choose a reason for hiding this comment

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

API_ROOT="${DIR}/cri/apis/v1alpha2" YEAR_TIME="" generateproto
}
main "$@"