Implement Action Versioning #4986

jiangpengcheng · 2020-09-28T01:43:46Z

Description

Add action versioning feature for actions

Design

Design document: https://cwiki.apache.org/confluence/display/OPENWHISK/Action+Versioning

Newly created/updated actions will include a version in their document id, like whisk.system/hello@0.0.2, so they will not replace old versions.

And to be compatible with current actions whose id doesn't contain version info, I added a new view action-versions in CouchDB which can use action's fullyQualifiedName to get all its versions and related ids, and it will return the default version of the action when includeDocs=true, so with one query request, system can get the default version(optional) and version-ids mappings for an action:

function (doc) {
  var isAction = function (doc) { return (doc.exec !== undefined) };
  if (isAction(doc)) try {
    var value = {
      _id: doc.namespace + "/" + doc.name + "/default",
      namespace: doc.namespace,
      name: doc.name,
      version: doc.version,
      docId: doc._id
    };
    emit([doc.namespace + "/" + doc.name], value);
  } catch (e) {}
}

There is a new annotation field publish for action, action owners can set it to false for test-only purpose to make sure users can not get/invoke non-published versions, and set it to true when action is well developed

Before update/get/invoke/delete an action, this view will be queried first to get the docId of action, and then using the docId to fetch action from database:

create/update:

if defaultVersion is passed and is a valid SemVer, system won't create a new action but just create/update the default version entry for this action with $fullyqualifiedName(false)/default as docId. if defaultVersion is not passed, goto step2
system will query action-versions view, and if version count exceed max allowed value, reject request. if not exceed, goto step3
get a suitable docid to update, if default version is set before, return default version's id, else, return latest version's id, goto step4; if no version exist at all, return None, goto step5.
create a new action based on old action got in step3 and user's input, with version = version.upPatch
create a new action based on user's input, with version=0.0.1

get:

if showVersions is true, return versions list of the action, else, goto step2:

case class WhiskActionVersionList(namespace: EntityPath,
                                  name: EntityName,
                                  versionMappings: Map[SemVer, DocId],
                                  defaultVersion: Option[SemVer])

get docId for the given action'name and version parameter, if version is not provided, the default or latest version(when default version is not set) will be choosed, then fetch action using this docId

invoke:

get docId for the given action'name and version parameter, if no version is specified, the default or latest version will be choosed
pass the docId to ActivationMessage
invoker can get action using docId passed with ActivationMessage directly, so it doesn't need to care about how to transform version to docId

delete:

if version is provided, delete related version, else, goto step2
get version list of action using design doc action-versions, goto step3
if there are multiple versions exist and deleteAll=false, reject request, else goto step4
delete all versions, and return the latest version of action

With this view, the old style of docId for actions is also compatible with this PR

And for performance reason, there is a cache layer for querying this view from database

Other than actions, there is also some tiny changes to trigger and rule, since in current master, trigger, rule and action can not using same name by follow same docId style, while with this PR, actions will use a different way for their docId(name + version), so triggers and rules may use same name with some actions.

So before save trigger and rule, it will also fetch the action-versions view first, and if there are some docIds returned, which means that entity name is occupied, openwhisk should return a Conflict error with message Resource by this name exists but is not in this collection.

TODO

support action versioning for webactions
support action versioning for sequence actions and conductor actions
support action versioning for trigger and rule

Related issue and scope

I opened an issue to propose and discuss this change ([Discussion] Action Versioning #4580)

My changes affect the following components

Types of changes

Bug fix (generally a non-breaking change which closes an issue).
Enhancement or new feature (adds new functionality).
Breaking change (a bug fix or enhancement which changes existing behavior).

Checklist:

I signed an Apache CLA.
I reviewed the style guides and followed the recommendations (Travis CI will check :).
I added tests to cover my changes.
My changes require further changes to the documentation.
I updated the documentation where necessary.

jiangpengcheng · 2020-09-28T01:45:05Z

common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala

@@ -143,6 +149,8 @@ case class WhiskAction(namespace: EntityPath,
  require(exec != null, "exec undefined")
  require(limits != null, "limits undefined")

+  override def docid = DocId(fullyQualifiedName(true).asString)


actions will add @version to their docIds

jiangpengcheng · 2020-09-28T01:46:30Z

common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala

@@ -106,8 +110,10 @@ abstract class WhiskActionLike(override val name: EntityName) extends WhiskEntit
      "annotations" -> annotations.toJson)
 }

-abstract class WhiskActionLikeMetaData(override val name: EntityName) extends WhiskActionLike(name) {
+abstract class WhiskActionLikeMetaData(override val name: EntityName, val docId: DocId) extends WhiskActionLike(name) {


ActionMetaData will get docId from database instead of generating it in code for compatible reason

jiangpengcheng · 2020-09-28T01:47:40Z

common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala

+  }
+
+  // delete cache
+  def deleteCache(action: FullyQualifiedEntityName)(implicit transId: TransactionId,


delete cache when any action is created/updated/deleted

Is there any possibility that this call fails which results in inconsistent data in the cache with what's in the db?

sorry for late response, there were other things seized me

the failure can happen at remote cache invalidation step, since it's using kafka, this is kind of critical, I need to think more

I override the evictionPolicy: EvictionPolicy = WriteTime, so event there is an error in kafka while delete cache, versions data will eventually consistent

jiangpengcheng · 2020-09-28T01:49:55Z

core/controller/src/main/scala/org/apache/openwhisk/core/controller/ApiUtils.scala

@@ -392,7 +392,7 @@ trait WriteOps extends Directives {
    onComplete(factory.get(datastore, docid) flatMap { entity =>
      confirm(entity) flatMap {
        case _ =>
-          factory.del(datastore, entity.docinfo) map { _ =>
+          factory.del(datastore, docid.asDocInfo(entity.rev)) map { _ =>


to compatible with old docId style

style95

It would be also great to share this PR to the dev list especially in terms of motivation, design, and the current approach in this PR.

style95 · 2020-09-28T15:19:02Z

common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala

@@ -548,8 +632,12 @@ object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[
      wp flatMap { resolvedPkg =>
        // fully resolved name for the action
        val fqnAction = resolvedPkg.fullyQualifiedName(withVersion = false).add(actionName)
+        val action = WhiskActionVersionList.get(fqnAction, entityStore).flatMap { result =>


I am still unsure about having two separate calls to fetch a certain version of the action.
I haven't tried this but there are some tricks to sort data like https://stackoverflow.com/a/41959344

If we can sort the data using the views we may have something like this with one list query:

actionA@0.0.3 actionA@0.0.2 actionA@0.0.1

Then we might be able to get the latest action metadata with the limit=1 parameter.

actionA@0.0.3

Also, if a DocId includes the version information, we can directly fetch a certain version even if it is old.

It is still important to provide a feature in a backward-compatible way, if this is designed in this way for backward compatibility, I am inclined not to let the design be changed because of the backward compatibility. Instead, we can provide other options such as a feature flag with a way to easily migrate existing data.

I believe the semantics or behavior of the feature should not be affected because of the backward compatibility.

It might be hard to sort since you'll have to follow semver versus just being able to do a lexicographical sort. It should be able to be done though in the view definition I think?

But yea keep in mind:
actionA@0.0.21
actionA@0.0.9

might not get sorted correctly if you don't have semver parsing in your sort

hm, maybe we can try use view key like [ "whisk.system/hello", 0, 0, 1 ]? I will check it

seems that it cannot reuse many current codes if do this way

style95 · 2020-09-28T15:34:04Z

common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala

@@ -548,8 +632,12 @@ object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[
      wp flatMap { resolvedPkg =>
        // fully resolved name for the action
        val fqnAction = resolvedPkg.fullyQualifiedName(withVersion = false).add(actionName)
+        val action = WhiskActionVersionList.get(fqnAction, entityStore).flatMap { result =>
+          val docId = result.matchedDocId(version).getOrElse(fqnAction.toDocId)
+          WhiskAction.get(entityStore, docId)


In case no version is specified, how does it fetch the latest version?

Have you thought about doing a default version rather than defaulting to latest version? That would be more work though because you have to add maintaining a default version so I'm fine with it defaulting to latest and that can be an added feature later. So long as as you just asked, old actions are backwards compatible and get picked up as latest version if there are no versions for the action.

@bdoyle0182 That sounds good to me.
Currently, it is too easy to update the action and it is immediately exposed.
In the production system, it is prone to do a mistake even if a proper CI is settled.

So I imagined having multiple versions with multiple routing just like blue-green or canary deployment and I believe the feature in this PR would be the baseline for it.
But at the same time, we can also let users specify and fix a certain version to be exposed, and make them change the exposed version for the safety.

yes exactly, having a client specified default version allows them to follow a safe release process. They can deploy their function, run tests on it, and then update the default version to start accepting traffic when they're ready. And then can also easily rollback by changing the default version back.

of course if all of your invoke requests can include the version you can do all of that without needing to maintain a default version, but not every client use case may have that level of control over their incoming traffic to be able to update the version on all invoke requests.

As I said though I think it's a great follow up feature since it's additional work to store. Just defaulting to a latest version is already much better for deployment management than what exists.

then we need to store that default version for each action and fetch it before invoke/get an action

how about adding a new field or an annotation field to action named publish which default to true if not exist, when user invoke/get an action without an version parameter, the latest and published action will be used, if version parameter is provided, then ignore this publish field, so users can test(invoke/get) with no-published versions

that would provide the functionality needed I think

bdoyle0182 · 2020-09-28T17:49:06Z

so many exciting PR's at once :)

bdoyle0182 · 2020-10-01T05:57:27Z

core/controller/src/main/scala/org/apache/openwhisk/core/controller/Actions.scala

@@ -216,11 +224,23 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
          case _ => entitlementProvider.check(user, content.exec)
        }

+        val latestDocId = WhiskActionVersionList.get(entityName, entityStore).map { result =>


This seems to be reused everywhere. Can we make this a future function we can just call to get the latest doc id?

yes, I will update

bdoyle0182 · 2020-10-05T04:56:50Z

Can we add creating a max version limit to todo? This is especially important if not supplying a version creates a new version on every action post, users will end up creating a lot of versions. Should be easy to have the check if we already have the action version view.

jiangpengcheng · 2020-10-09T01:49:22Z

Can we add creating a max version limit to todo? This is especially important if not supplying a version creates a new version on every action post, users will end up creating a lot of versions. Should be easy to have the check if we already have the action version view.

I will implement it in this PR

bdoyle0182 · 2020-10-16T18:37:01Z

core/controller/src/main/scala/org/apache/openwhisk/core/controller/Actions.scala

+                    make(user, entityName, request)
+                  },
+                  postProcess = Some { action: WhiskAction =>
+                    // delete oldest version when created successfully


Can we have an operator level configuration where if you are at the max limit of versions, you either do this delete the oldest version or reject the create request and require the user to delete an old version before creating a new one?

good idea, I will update

add a deleteOld parameter to create method

bdoyle0182 · 2020-10-16T19:13:42Z

core/controller/src/main/scala/org/apache/openwhisk/core/controller/Actions.scala

+                    WhiskAction.del(entityStore, entity.docinfo).map(_ => entity)
+                  })
+                else
+                  results.versions.values


I think this is a little scary to delete all versions if a version accidentally isn't supplied. Should there be a delete all parameter flag otherwise return an error?

Also is it possible that you have a list of versions and then from backwards compatibility still have an action with no version? Does that result in the action with no version to get left over after the delete?

ok

since it will fetch a version-id mappings at first, so even for action with no version, we can get the id of it and delete action by id

bdoyle0182 · 2020-10-16T21:14:03Z

"if there is no docId, just created a new action based on user's input and save it"

Does this imply that if an entirely new doc id / action is created without a version supplied, that it will be created without a version? Or is it created as 0.0.1 so that all new actions post update will be created with a version?

bdoyle0182 · 2020-10-16T21:15:33Z

common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala

+}
+
+object WhiskActionVersionList extends MultipleReadersSingleWriterCache[WhiskActionVersionList, DocInfo] {
+  lazy val viewName = WhiskQueries.entitiesView(collection = "action-versions").name


nit: should use a constant for "action-versions"

bdoyle0182 · 2020-10-16T21:41:30Z

LGTM after my comments. I don't know what the approval process for such a substantial feature like this should be, but I will approve once one or two more people have gone through this. I'm impressed you were able to implement a feature like this and maintain backwards compatibility without the code getting out of control. This is a feature we've wanted for years.

One thing to add about backwards compatibility, could we create a migration script that adds version @0.0.1 to the db and version view for every non-versioned action. Once upgraded, we would like to do something like that so that we don't have a mixture of the two types of data to deal with. And if such a script is provided, we can remove the backwards compatibility support in a subsequent release to keep the codebase clean and direct operators to use the script once upgraded past this commit.

jiangpengcheng · 2020-10-26T02:06:48Z

"if there is no docId, just created a new action based on user's input and save it"

Does this imply that if an entirely new doc id / action is created without a version supplied, that it will be created without a version? Or is it created as 0.0.1 so that all new actions post update will be created with a version?

a 0.0.1 will be used

jiangpengcheng · 2020-10-29T08:50:54Z

common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala

+      // there may be a chance that database is updated while cache is not, we need to invalidate cache and try again
+      if (docId.isEmpty && tryAgain) {
+        WhiskActionVersionList.removeId(cacheKey(action))
+        getMatchedDocId(action, version, datastore, false)


delete cache in memory first and try again to avoid the inconformity between database and cache

jiangpengcheng · 2020-10-29T09:09:16Z

code looks kind of ugly to make it compatible with old style, I will write a script to copy current docs to new docs

jiangpengcheng · 2020-11-19T07:35:16Z

I wrote a wiki in https://cwiki.apache.org/confluence/display/OPENWHISK/Action+Versioning to help understanding this PR

ningyougang · 2021-04-30T02:41:16Z

will provide version list query feature with cli tool in future? e.g. wsk actionVersion $action list or wsk action version $action list
it is better to update the document
- Change the create action flow's withDefaultVerstion -to withVersionParameter
- Add the publish logic to the document

upgle · 2021-05-06T03:05:03Z

@jiangpengcheng please resolve conflicts

Deklin · 2021-05-06T07:41:11Z

Just to add some comments to this.
It would be beneficial to be able to run an action with a @Version tag as specified

e.g. /helloworld@1.0.0

However it is also important that /helloworld and perhaps helloworld/@latest always run the latest published version. I believe other FAAS services like fission do versioning this way and it makes contractual binding from consumers much easier.

I can either point to a specific version if I am in a strictly controlled environment, or can always point to @latest to get the latest published version of the code.

--edit

It does look like the design document handles this scenario in the invoker :) Thank you

This reverts commit 49396bf.

1. provide a migrate script to copy old actions to new docs with new id 2. use a separate entity to save the default version for action

bdoyle0182 · 2021-06-23T22:26:33Z

Are there any open concerns with this PR? Since this is backwards compatible, I don't have any concerns with finally merging this to master once built. The only thing I can think of would be to wait until there's support for all action types, but I'm not sure if that's worth waiting for. This is a very important feature to keep up with other FaaS offerings.

jiangpengcheng · 2021-06-24T01:23:53Z

Are there any open concerns with this PR? Since this is backwards compatible, I don't have any concerns with finally merging this to master once built. The only thing I can think of would be to wait until there's support for all action types, but I'm not sure if that's worth waiting for. This is a very important feature to keep up with other FaaS offerings.

thanks for review, I'm waiting for all new scheduler prs get merged, and add versioning support for them, after then I will send an email to mail list for a discussion

bdoyle0182 · 2022-01-27T19:43:41Z

Bumping this pr since all pr's for the new scheduler have been merged in. It would be great to this feature in now finally. It's a big deal to have

jiangpengcheng · 2022-01-28T00:18:14Z

@bdoyle0182 thanks for reminding, I will handle this after Lunar New Year vacation

bdoyle0182 · 2022-08-05T17:24:46Z

It would be great to finally get this feature in now that the scheduler is open sourced.

One idea I had was how can we handle dev / snapshotting versions? Since right now it seems like it will only support strict semantic versioning of /d*/./d*/./d. Or if it's something we really need?

bdoyle0182 · 2023-02-15T19:12:03Z

resolved conflicts

fix bad conflict resolution

bdoyle0182 · 2023-02-16T00:14:58Z

One Additional ToDo on top of what's listed in the description, add view mapper for actions-versions view to the MongoDb view mappers

jiangpengcheng commented Sep 28, 2020

View reviewed changes

jiangpengcheng added the wip label Sep 28, 2020

style95 reviewed Sep 28, 2020

View reviewed changes

bdoyle0182 reviewed Oct 1, 2020

View reviewed changes

jiangpengcheng force-pushed the feature/implement_code_versioning branch from f02111f to e5c6af0 Compare October 9, 2020 09:08

jiangpengcheng removed the wip label Oct 14, 2020

jiangpengcheng changed the title ~~[WIP]Implement action versioning~~ Implement action versioning Oct 14, 2020

bdoyle0182 reviewed Oct 16, 2020

View reviewed changes

jiangpengcheng force-pushed the feature/implement_code_versioning branch from 34938f8 to f284a6c Compare October 29, 2020 08:44

jiangpengcheng commented Oct 29, 2020

View reviewed changes

jiangpengcheng force-pushed the feature/implement_code_versioning branch from f6d3b65 to e3c44da Compare November 13, 2020 10:08

style95 requested review from ningyougang and upgle April 29, 2021 06:21

jiangpengcheng force-pushed the feature/implement_code_versioning branch from ad99088 to 4df5178 Compare May 8, 2021 06:18

jiangpengcheng added 16 commits June 3, 2021 08:57

Add deleteAll parameter for action#remove

d1b2c54

Add deleteOld parameter for action#create

2b1c4e8

Revert "Allow to replace specify version"

e030b4b

This reverts commit 49396bf.

add default version feature

4d0e220

Some updates:

af0d7c2

1. provide a migrate script to copy old actions to new docs with new id 2. use a separate entity to save the default version for action

Fix format style

5e4bc9f

Fix test

4dc8147

Remove some useless code

ef1711c

Fix bug

5278fdc

Fix rebase error

f2b348a

Fix tests error

becdb02

Fix tests

6fe1d7f

Use version mappings to get doc id from an version

a3939ed

Update some return errors

fc564c9

Rebase master

9fad6e3

Fix rebase errors

0205d2e

jiangpengcheng force-pushed the feature/implement_code_versioning branch from acdde25 to 0205d2e Compare June 3, 2021 03:01

Merge branch 'master' into feature/implement_code_versioning

54cb3de

bdoyle0182 and others added 2 commits February 15, 2023 11:53

Update Actions.scala

6a41968

fix bad conflict resolution

fix compilation w/ new scheduler code

482c05e

bdoyle0182 changed the title ~~Implement action versioning~~ Implement Action Versioning Feb 16, 2023

fix versioned action read on fpcs invoker

cc56908

style95 added the stale old issue which needs to validate label May 31, 2023

Implement Action Versioning #4986

Are you sure you want to change the base?

Implement Action Versioning #4986

Conversation

jiangpengcheng commented Sep 28, 2020 • edited

Description

Design

TODO

Related issue and scope

My changes affect the following components

Types of changes

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jiangpengcheng Sep 28, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

style95 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bdoyle0182 commented Sep 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bdoyle0182 commented Oct 5, 2020

jiangpengcheng commented Oct 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bdoyle0182 commented Oct 16, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bdoyle0182 commented Oct 16, 2020

jiangpengcheng commented Oct 26, 2020

Choose a reason for hiding this comment

jiangpengcheng commented Oct 29, 2020 • edited

jiangpengcheng commented Nov 19, 2020

ningyougang commented Apr 30, 2021 • edited

upgle commented May 6, 2021

Deklin commented May 6, 2021 • edited

bdoyle0182 commented Jun 23, 2021 • edited

jiangpengcheng commented Jun 24, 2021

bdoyle0182 commented Jan 27, 2022

jiangpengcheng commented Jan 28, 2022

bdoyle0182 commented Aug 5, 2022

bdoyle0182 commented Feb 15, 2023

bdoyle0182 commented Feb 16, 2023

jiangpengcheng commented Sep 28, 2020 •

edited

jiangpengcheng Sep 28, 2020 •

edited

jiangpengcheng commented Oct 29, 2020 •

edited

ningyougang commented Apr 30, 2021 •

edited

Deklin commented May 6, 2021 •

edited

bdoyle0182 commented Jun 23, 2021 •

edited