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

The namespace of workflow environment can not be configured #160

Closed
xiekeyang opened this issue Jul 5, 2018 · 10 comments
Closed

The namespace of workflow environment can not be configured #160

xiekeyang opened this issue Jul 5, 2018 · 10 comments

Comments

@xiekeyang
Copy link
Contributor

xiekeyang commented Jul 5, 2018

@erwinvaneyk I encounter some questions when configure the workflow environment:

environment namespace

The namespace of workflow environment seems not able to be configured. I changed it to fission-function instead default, and then the invocation of workflowed function will be hung up. (default namespace is OK)

Not support customized parameters in config spec


~~~I tried to add resource limitation to config spec, however it seems not work in created deployment.~~~

~~~The config like below (add `resource` items):~~~

```yaml
apiVersion: fission.io/v1
kind: Environment
metadata:
  name: workflow
  namespace: default
spec:
  version: 2
  runtime:
    image: fission/workflow-env
    container:
      env:
      - name: ES_NATS_URL
        value: "nats://[email protected]:4222"
      - name: ES_NATS_CLUSTER
        value: "fissionMQTrigger"
      - name: FNENV_FISSION_CONTROLLER
        value: "http://controller.fission"
      - name: FNENV_FISSION_EXECUTOR
        value: "http://executor.fission"
      resources:
        limits:
          cpu: 400m
          memory: 256Mi
        requests:
          cpu: 200m
          memory: 128Mi
  builder:
    image: fission/workflow-build-env
    command: "defaultBuild"
  allowedFunctionsPerContainer: infinite
```

This is my fault, I put the resource config to wrong space.
@xiekeyang
Copy link
Contributor Author

The correct config template should be

apiVersion: fission.io/v1
kind: Environment
metadata:
  name: workflow
  namespace: default
spec:
  version: 2
  TerminationGracePeriod: 5
  resources:
    limits:
      cpu: 200m
      memory: 256Mi
    requests:
      cpu: 100m
      memory: 128Mi
  runtime:
    image: fission/workflow-env
    container:
      env:
      - name: ES_NATS_URL
        value: "nats://[email protected]:4222"
      - name: ES_NATS_CLUSTER
        value: "fissionMQTrigger"
      - name: FNENV_FISSION_CONTROLLER
        value: "http://controller.fission"
      - name: FNENV_FISSION_EXECUTOR
        value: "http://executor.fission"
      resources:
        limits:
          cpu: 400m
          memory: 256Mi
        requests:
          cpu: 200m
          memory: 128Mi
  builder:
    image: fission/workflow-build-env
    command: "defaultBuild"
  allowedFunctionsPerContainer: infinite

@erwinvaneyk
Copy link
Member

Hi @xiekeyang - just to summarize our discussion on Slack here a bit:

  • The environment namespace is indeed a valid issue, we should look into where we make assumptions about this specific namespace in the codebase
  • For the other issue, great that you found it yourself 😄

@erwinvaneyk erwinvaneyk changed the title Some questions about workflow environment The namespace of workflow environment can not be configured Jul 13, 2018
@xiekeyang
Copy link
Contributor Author

xiekeyang commented Jul 20, 2018

Improvement Proposal of Selectable Namespace

Problem:

Current workflow functions only allow configuration of task function name, with a fixed namespace of default.

That don’t allow the functions under other ns to add to workflow task.

Idea:

Add a functionns parameter to identify its task function namespace, to Workflow Spec, for example:

apiVersion: 1
output: echohello
  
tasks:
  echohello:
    run: hello                 
    functionns: my-namespace

And composed workflow template:

apiVersion: 1 
output: echocompose
tasks: 
  echohello:
    run: hello
    functionns: my-namespace
  echoworld:
    run: world
    functionns: my-namespace
  echocompose:
    run: compose
    inputs:
      hello: "{ output('echohello') }"
      world: "{ output('echoworld') }"
    requires: 
    - echohello
    - echoworld

The empty functionns means default.

Use entire struct FnRef{runtime, id, ns} to be key to resolve task. That can identify all functions in all fission deployments scope.

FnRef is formatted to like fission://my-namespace/hello.

Move FunctionRef member from types.TaskSpec, like:

type TaskSpec struct {
    Inputs      map[string]*TypedValue
    Requires    map[string]*TaskDependencyParameters
    Await       int32
    Output      *TypedValue
}

The task spec and task identifier are managed by map(key/value): map[FnRef] *TaskSpec.

@erwinvaneyk WDYT? Thanks.

@xiekeyang
Copy link
Contributor Author

xiekeyang commented Jul 20, 2018

Another Story: Current run syntax allow filling some rules (noop, compose…) or function name, which seems a little confused. We had better to refactor it strictly and clearly to identify only runtime rule, and create a new member like function to identify task function name.

It is referred to serverless white paper.

@xiekeyang
Copy link
Contributor Author

I mistook above because that proto syntax don't allow structured mapping key. And yaml spec template should not use functionns member. The template should like:

apiVersion: 1
output: echohello
  
tasks:
  echohello:
    run: "fisison://namespace/hello"

or

apiVersion: 1
output: echohello
  
tasks:
  echohello:
    run: "namespace/hello"

But I'm not sure if we really need define runtime fission word here.

@xiekeyang
Copy link
Contributor Author

We had thought to set fnref key template as fission://<namespace>/<fn-name> for functions running on deployment fission, and internal://compose for internal function. However, this file should be filed by customers. Usually it is impossible for customer to be aware the name fission of specific deployment. They might only know FISSION_URL, FISSION_ROUTER.

@erwinvaneyk
Copy link
Member

erwinvaneyk commented Jul 25, 2018

It is referred to serverless white paper.

The link doesn't reference the white paper? I am interested; could you give me the correct link? 😁

But I'm not sure if we really need define runtime fission word here.

Just for future reference, on slack we settled on: if undefined runtime -> check internal first for function -> check fission for function

However, this file should be filed by customers. Usually it is impossible for customer to be aware the name fission of specific deployment. They might only know FISSION_URL, FISSION_ROUTER.

That is an interesting issue, that users do not know the deployment runtime in advance. I am not yet sure what the best solution for this is.

What about adding a build step that renames fission:// to fission-CLIENTID://? That way the instructions can remain simple, and the user is not constraining their workflow to their specific deployment. In other words, I think, we need to add some multi-tenancy component to FW; not necessarily exposed to the user.

@xiekeyang
Copy link
Contributor Author

@erwinvaneyk

Sorry the correct link should be https://github.com/cncf/wg-serverless/tree/master/whitepaper, which is CNCF release.

@xiekeyang
Copy link
Contributor Author

This should be closed by #176. As previous discussion with @erwinvaneyk , we still left some topic about resolver of tasks, including their runtime. I would like to open a new issue to go on this.

@erwinvaneyk
Copy link
Member

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants