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

doc: add document pouch_with_upgrade.md #2346

Merged

Conversation

HusterWan
Copy link
Contributor

Signed-off-by: Michael Wan zirenwan@gmail.com

Ⅰ. Describe what this PR did

Add introduction document for upgrade interface.

You can get information about the design of upgrade interface, of cause about how to use the upgrade cli tools from the document.

Ⅱ. Does this pull request fix one issue?

fixes #968

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)

add a doc

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@codecov
Copy link

codecov bot commented Oct 23, 2018

Codecov Report

Merging #2346 into master will decrease coverage by 14.91%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #2346       +/-   ##
===========================================
- Coverage   67.85%   52.94%   -14.92%     
===========================================
  Files         265      265               
  Lines       18211    18211               
===========================================
- Hits        12357     9641     -2716     
- Misses       4421     7499     +3078     
+ Partials     1433     1071      -362
Flag Coverage Δ
#criv1alpha1test ?
#criv1alpha2test ?
#integrationtest 39.53% <ø> (+0.04%) ⬆️
#nodee2etest ?
#unittest 25.31% <ø> (ø) ⬆️
Impacted Files Coverage Δ
cri/v1alpha1/cri_types.go 0% <0%> (-100%) ⬇️
cri/v1alpha2/cri_types.go 0% <0%> (-100%) ⬇️
cri/v1alpha2/service/cri.go 0% <0%> (-89.48%) ⬇️
cri/v1alpha1/service/cri.go 0% <0%> (-84.62%) ⬇️
cri/stream/remotecommand/websocket.go 0% <0%> (-81.54%) ⬇️
cri/stream/remotecommand/exec.go 0% <0%> (-80%) ⬇️
daemon/containerio/streams.go 20.68% <0%> (-79.32%) ⬇️
cri/v1alpha2/server.go 0% <0%> (-79.2%) ⬇️
cri/v1alpha1/server.go 0% <0%> (-79.2%) ⬇️
daemon/containerio/cri_log_file.go 11.76% <0%> (-76.48%) ⬇️
... and 45 more


## Introduction

Most containers within Alibaba are used in rich container mode. Some containers among the rich containers in traditional virtual machine O&M mode are still stateful. Updates and upgrades of stateful services are frequently performed daily operations within Alibaba. For the container technology that delivers images, the container operations corresponding to service update and upgrade include two steps: deletion of old image container and creation of the new image container.
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 it is a good way to mention Alibaba in the general technical documents. Since what you invent is quite a general feature for the whole industry, explicit explanation for you feature is enough to make it clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

great advise

@allencloud
Copy link
Collaborator

I think upgrade feature is general for all kinds of containers, not only stateful containers with storage, but also other ip-fixed containers. So could we ignore the rich container mode in the upgrade document. With mentioning rich container, it will mislead end-users that only rich container mode needs upgrade feature you invented. @HusterWan WDYT?


* Customer Case 2: In a middleware service, the service registration mode is used. That is, the IP addresses of the scaled-up containers must be registered in the server list; otherwise, the containers are invalid. The new containers must inherit the IP addresses of old containers every time the service containers are upgraded; otherwise, the new services are invalid.

## Design
Copy link
Collaborator

Choose a reason for hiding this comment

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

With the design, could you add a graph to illustrate it clearly? 😆

Copy link
Contributor Author

Choose a reason for hiding this comment

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

already added


## Design

Before using the `upgrade` interface, we should figure out which parameter of container you want to update ? If you want to update a container's [Resources](../../apis/types/resources.go), I think you should use the `update` interface. If you just want to update a container's image and inherit the volumes and network from the old container, now you should use the `upgrade` interface.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here you need to the following part outlined:

Difference between UPDATE and UPGRADE

blabla

}
```

It is enough to update a container from a old image to a new one now. If anyone has new situation need to add more parameters into `ContainerUpgradeConfig`, we can create a new issue to discuss it.
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/a old/an old

When you are using upgrade feature, please try to use word upgrade rather than update.
I think this would reduce something ambiguous.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you are right


### About core logic

#### Which cmd should be used to create the new container when upgrade
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/cmd/CMD

Which CMD is used when upgrading a container?


#### rollback of upgrade failure

When occurred any error during upgrade, we should have the ability to rollback the upgrade and recover the old container just like it before.
Copy link
Collaborator

Choose a reason for hiding this comment

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

When any errors occur during upgrade procedure, pouchd will automatically rollback the whole upgrade operation and the old container will be recovered just like it was.


When occurred any error during upgrade, we should have the ability to rollback the upgrade and recover the old container just like it before.

### Show the upgrade feature
Copy link
Collaborator

Choose a reason for hiding this comment

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

Quick Start

or

Example

@HusterWan HusterWan force-pushed the zr/add-upgrade-doc branch 3 times, most recently from 7ff83dc to 549748b Compare October 24, 2018 04:17
@HusterWan
Copy link
Contributor Author

I have updated this doc, PTAL thanks @allencloud


The following architecture graph shows how to upgrade a container:

![pouch_with_upgrade](../static_files/pouch_with_upgrade.png)
Copy link
Collaborator

@allencloud allencloud Oct 25, 2018

Choose a reason for hiding this comment

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

why not take three steps to explain this graph? 😃

first, inherit container attributes from the old container in pouchd's memory;
second, replace container image via ......
third, just start the newly constructed command if needed.

Otherwise, people may not have sense of it. WDYT? @HusterWan

Sometimes, only sentence is not convincing. If only a graph, still not so convincing.
However if we combine sentences and graphs, it appears so clear.

@allencloud
Copy link
Collaborator

LGTM until the graph explanation part finishes. @HusterWan

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Oct 26, 2018
@HusterWan
Copy link
Contributor Author

LGTM until the graph explanation part finishes. @HusterWan

@allencloud I will updated right now

@HusterWan HusterWan force-pushed the zr/add-upgrade-doc branch 2 times, most recently from 9ef8a80 to 9acb912 Compare October 26, 2018 08:46
Signed-off-by: Michael Wan <zirenwan@gmail.com>
@allencloud
Copy link
Collaborator

Thanks for your update. @HusterWan
Merging...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
areas/docs LGTM one maintainer or community participant agrees to merge the pull reuqest. size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[doc required] need a document to explain upgrade function
3 participants