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 get envs #2163

Merged
merged 1 commit into from
Aug 30, 2018
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
698 changes: 380 additions & 318 deletions cri/apis/v1alpha2/api.pb.go

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions cri/apis/v1alpha2/api.proto
Original file line number Diff line number Diff line change
Expand Up @@ -919,6 +919,8 @@ message ContainerStatus {
LinuxContainerResources resources = 101;
// QuotaId of the container
string quota_id = 102;
// List of environment variable to set in the container.
repeated KeyValue envs = 103;
}

message Volume {
Expand Down
1 change: 1 addition & 0 deletions cri/v1alpha2/cri.go
Original file line number Diff line number Diff line change
Expand Up @@ -785,6 +785,7 @@ func (c *CriManager) ContainerStatus(ctx context.Context, r *runtime.ContainerSt
Volumes: parseVolumesFromPouch(container.Config.Volumes),
Resources: parseResourcesFromPouch(resources, diskQuota),
QuotaId: container.Config.QuotaID,
Envs: parseEnvsFromPouch(container.Config.Env),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I do not think we should add Pouch in the function naming. Please change a proper one. @starnop

Copy link
Contributor

Choose a reason for hiding this comment

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

@starnop Maybe we should rename the functions like parseEnvsFromPouch, parseResourcesFromPouch in another PR, they all do the same thing :)

@allencloud WDYT?

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 will do that in the next pull request.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I still insist that we could finish in the pull request. How about convertEnvsForCRI or convertContainerEnvForCRI?

}

return &runtime.ContainerStatusResponse{Status: status}, nil
Expand Down
16 changes: 16 additions & 0 deletions cri/v1alpha2/cri_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -1087,6 +1087,22 @@ func parseVolumesFromPouch(containerVolumes map[string]interface{}) map[string]*
return volumes
}

// parseEnvsFromPouch parse Envs from []string to []*runtime.KeyValue
func parseEnvsFromPouch(pouchEnvs []string) (criEnvs []*runtime.KeyValue) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

you are not parsing, you just did the translation thing.

for _, env := range pouchEnvs {
runtimeEnv := &runtime.KeyValue{}
if strings.Contains(env, "=") {
envItem := strings.SplitN(env, "=", 2)
runtimeEnv.Key = envItem[0]
runtimeEnv.Value = envItem[1]
} else {
runtimeEnv.Key = env
}
criEnvs = append(criEnvs, runtimeEnv)
}
return
}

// CNI Network related tool functions.

// toCNIPortMappings converts CRI port mappings to CNI.
Expand Down
66 changes: 66 additions & 0 deletions cri/v1alpha2/cri_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1736,6 +1736,72 @@ func Test_parseVolumesFromPouch(t *testing.T) {
}
}

func Test_parseEnvsFromPouch(t *testing.T) {
type args struct {
pouchEnvs []string
}
tests := []struct {
name string
args args
wantCriEnvs []*runtime.KeyValue
}{
{
name: "Normal Test",
args: args{
pouchEnvs: []string{"key1=value1", "key2=value2"},
},
wantCriEnvs: []*runtime.KeyValue{
{
Key: "key1",
Value: "value1",
},
{
Key: "key2",
Value: "value2",
},
},
},
{
name: "Multiple Equals Sign Test",
args: args{
pouchEnvs: []string{"key1=value1", "key2=foo=value2"},
},
wantCriEnvs: []*runtime.KeyValue{
{
Key: "key1",
Value: "value1",
},
{
Key: "key2",
Value: "foo=value2",
},
},
},
{
name: "No Equals Sign Test",
args: args{
pouchEnvs: []string{"key1=value1", "key2"},
},
wantCriEnvs: []*runtime.KeyValue{
{
Key: "key1",
Value: "value1",
},
{
Key: "key2",
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if gotCriEnvs := parseEnvsFromPouch(tt.args.pouchEnvs); !reflect.DeepEqual(gotCriEnvs, tt.wantCriEnvs) {
t.Errorf("parseEnvsFromPouch() = %v, want %v", gotCriEnvs, tt.wantCriEnvs)
}
})
}
}

func Test_generateMountBindings(t *testing.T) {
type args struct {
mounts []*runtime.Mount
Expand Down
21 changes: 15 additions & 6 deletions docs/kubernetes/pouch_cri_api_changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ Kubernetes Version: V1.10.0+
3. Support to the ability to acquire the volumes of the Container.
- Scenario:
- In the Upgrade process, the volumes of the new container needs to remain consistent with that of the old container, so the volumes of the old container needs to be read.

4. Support to the ability to acquire the envs of the Container.
- Scenario:
- In the Upgrade process, the env of the new container needs to remain consistent with that of the old container, so the envs of the old container needs to be read.

# The Changes Of CRI API

## UpdateContainerResources
Expand All @@ -38,7 +41,7 @@ Kubernetes Version: V1.10.0+

### Modification

+ Add the DiskQuota field in LinuxContainerResources, referring to the definition of Resources in [moby](https://github.com/moby/moby/blob/master/api/types/container/host_config.go) for better compatibility. The new fields are as follows:
+ Add the `DiskQuota` field in `LinuxContainerResources`, referring to the definition of Resources in [moby](https://github.com/moby/moby/blob/master/api/types/container/host_config.go) for better compatibility. The new fields are as follows:

```
type LinuxContainerResources struct {
Expand Down Expand Up @@ -149,12 +152,14 @@ message Ulimit {
+ Support to the ability to acquire the volumes of the Container.
+ Support to the ability to acquire the Resource of the Container.
+ Pass the quotaID generated when the container is created for disk reuse.
+ Support to the ability to acquire the envs of the Container.

### Modification

+ The ContainerStatus struct is used only in ContainerStatusResponse in CRI, so the volumes of the container can be obtained by directly adding volume field to the ContainerStatus struct.
+ The `ContainerStatus` struct is used only in `ContainerStatusResponse` in CRI, so the volumes of the container can be obtained by directly adding `volume` field to the `ContainerStatus` struct.
+ Add Resources field to support the retrieval of container's resource.
+ When get ContainerStatus, the return object of ContainerStatusResponse will contain the field of QuotaId .
+ When get ContainerStatus, the return object of `ContainerStatusResponse` will contain the field of `QuotaId` .
+ When get ContainerStatus, the return object of `ContainerStatusResponse` will contain the field of `Envs` .

```
// ContainerStatus represents the status of a container.
Expand All @@ -167,6 +172,8 @@ type ContainerStatus struct {
Resources *LinuxContainerResources `protobuf:"bytes,101,opt,name=resources" json:"resources,omitempty"`
// QuotaId of the container
QuotaId string `protobuf:"bytes,102,opt,name=quota_id,json=quotaId,proto3" json:"quota_id,omitempty"`
// List of environment variable to set in the container.
Envs []*KeyValue `protobuf:"bytes,103,rep,name=envs" json:"envs,omitempty"`
}
```

Expand All @@ -183,6 +190,8 @@ message ContainerStatus {
LinuxContainerResources resources = 101;
// QuotaId of the container
string quota_id = 102;
// List of environment variable to set in the container.
repeated KeyValue envs = 103;
}

message Volume {
Expand All @@ -197,7 +206,7 @@ message Volume {

### Modification

+ Add volumes field in the Image struct.
+ Add `volumes` field in the Image struct.

```
// Basic information about a container image.
Expand Down Expand Up @@ -230,7 +239,7 @@ message Image {

### Modification

+ LinuxContainerConfig contains LinuxContainerResources, which have changed in UpdateContainerResources().So after changing LinuxContainerResources, the Create process already supports the setting of DiskQuota.
+ `LinuxContainerConfig` contains `LinuxContainerResources`, which have changed in UpdateContainerResources().So after changing LinuxContainerResources, the Create process already supports the setting of DiskQuota.
+ For missing fields are as follows (not all):
+ NetPriority : Set network priorities
+ QuotaId : When creating container, pass parameters of the DiskQuota and QuotaId. (When QuotaId is -1, QuotaId will be automatically generated)
Expand Down