knative / func Goto Github PK

We currently have the cloudevents template, and would like to add support for an http handler as well.

We currently have a docker container which is created via workflows on pushes with version tags, but this should be expanded to also create a proper github release with prebuilt binaries for various platforms and architectures. Presently all resources are built into the binary, so a docker container is not strictly necessary as there are no ancillary files required.

The GH workflow for our releases sets draft: false. I wonder if we should create a release as a draft, and then manually update the release notes with a changelog of some kind.

Alternatively, we could use tooling like git-chglog to generate a changelog for the release and include that in the release description during the workflow.

Create a cluster services adapter which runs alongside functions, abstracting services offered by the cluster. At first this service should:

• Start on a well-known port in loopback, thus reusing service credentials and remaining itself implicitly local.
• Proffer a simple REST JSON API with outbound connections using full client libraries (usually gRPC).
• Provide hooks for integrating varying implementations based on environment
• Have provisions for starting in local development mode

Some of the advantages this architecture is expected to provide are:

• Future support of varying underlying cluster configurations (local development, Knative, others),
• The centralization of work related to complex integration rather than requiring re-implementation for each language.
• Provide runtime introspection without the need for client connections
• A simplified interface to function developers resulting in even optionally omitting any client libraries whatsoever
• The support of local development using a lightweight single-node implementation of features.
• Allowing function developers to configure their services entirely in the language of their choice.
• Retaining the function developer's ability to use logic during the initialization step, rather than relying on config file pre-processors.

We could write completion for the registry flag. The docker file config contains both registry URI and username.
See matejvasek@e903126#diff-1b934a39b47085a83d728ef700e18656R57.
@matejvasek

In addition to updating the completions to reflect the latest CLI in #81 , adding a custom completion for the --repository flag would be a great help for those who choose not to set it as an environment variable as is recommended.

To clarify, this would autocomplete the portion of a Function's Image which includes the Registry and Namespace
Docker Example: "docer.io/alice"
OpenShift: "baseDomain.tld/faas"
Quay: "quay.io/alice"

Execute tests under windows and osx

Import documentation from boson-project/faas-config.

Now that we have a means to actually perform a release, maybe we should release something! 🥇

On various places accros the project, the image repository is referred as namespace or FAAS_NAMESPACE. It could be confusing as user could get impression that we are talking about kubernetes namespace (I was mistaken as well) and as CLI help is suggesting.

https://github.com/boson-project/faas/blob/develop/docs/developers_guide.md#local-setup-and-configuration

$ faas create --help                                                                                                                                                                                               
Create a new Function, including initialization of local files and deployment.

Usage:
  faas create <name> [options] [flags]

Flags:
  -h, --help                help for create
  -i, --image string        Optional full image name, in form [registry]/[namespace]/[name]:[tag] for example quay.io/myrepo/project.name:latest (overrides --repository) - $FAAS_IMAGE
  -n, --namespace string    Override namespace into which the Function is deployed (on supported platforms).  Default is to use currently active underlying platform setting - $FAAS_NAMESPACE

What if we use image registry/repository nomenclature for the image name that we are going to build, eg. docker.io/johndoe? This way we can avoid the confusion with namespace.

While the client library does expect path to be explicitly passed, the CLI does not currently expose this as a flag, instead always defaulting to the current working directory.

All commands which use code on disk (defaulting to current-working directory) should accept a --path flag to allow affecting change on a directory other than the execution directory.
For example:

faas create go --path ~/src/example.com/admin

should be equivalent to

cd ~/src/example.com/admin && faas create go

File permissions are not preserved when initializing new funcitons on Windows.
This is very possibly a bug or missing feature in the pkger dependency. If this is indeed the case, we would ideally resove by submitting a PR.

Originally posted by @matejvasek in #49 (comment)

Currently there are two problems with the registry configuration. First, it is defaulted to an external registry (quay.io) and secondly the repositories created during the creation process are defaulted to private, and there appears no way to update this default behavior. Taken together, these create an unfortunate barrier to usage. Ideally, we would deafault to using a dedicated in-cluster registry for function container artifacts, with the ability to upgrade to using an external, third-party registry if desired. This can be accomplished by manually configuring a v2 Docker Registry mirror in cluster, which we can later encapsulate in the Functions Operator installation process.

It is unclear to me at this time what steps are necessary to expose the registry for direct-push, rather than rely entirely on its mirroring of Docker hub, but I assume it is possible because one can push directly to the registry when running locally. I presume it will require setting a public route, and perhaps placing it behind an Oauth proxy. We could also perhaps rely on opening a tunnel such as with kubectl proxy.

With this registry endpoint available, however, it can become the default option, with the --registry flag defaulted to empty, indicating faas should attempt to use the in-cluster registry accessible in-cluster.

The package.json file in the Node.js template has the two runtime dependencies in the devDependencies section, and an unnecessary dependency on cloudevents-sdk (which is built in to faas-js-runtime).

These should be moved to a dependencies stanza.

    "@redhat/faas-js-runtime": "0.2.0",
    "death": "^1.1.0",

And this should be removed altogether.

    "cloudevents-sdk": "^1.0.0",

The HTTP template has a similar issue. This should be changed to version 0.2.1 and moved to the dependencies stanza.

    "@redhat/faas-js-runtime": "boson-project/faas-js-runtime"

If I run this faas create command with details specifed in the parameters, I don't expect to get asked about the same information later in the prompt. I know there is -y parameter, but IMHO this shouldn't be needed for already specified details.

$ faas create test2.apps.zroubali.devcluster.openshift.com --runtime go --repository docker.io/zroubalik --trigger http  
                                                                                          
Path to project directory (/Users/zroubali/boson/test2):
Function project name (test2.apps.zroubali.devcluster.openshift.com):
Verbose logging (y/N): y
Runtime of source (go):
Function Trigger (http):
Repository for Function images (docker.io/zroubalik):
Override default deploy namespace (zroubalik):

The golang templates for events and http should include a README.md document to help developers get started

Testing the templates as part of our CI will probably help prevent us from getting bit at some point.

The build system should build its platform-specific binaries from a seperate build step, but with the default make path running go build without specifying architecture or OS such that it works with defaults from developer machines running on, for instance, ARM.

We currently release a binary for linux amd64. It would be nice to create a prebuilt binary for a few common OSs and architectures. Perhaps Linux and Darwin for both AMD and ARM to begin with, adding more as requested.

Update to use branch name 'main', as the default.

The build step presently uses the appsody binary for its implementation, and makes use of custom "stacks" referenced through the repository boson-project/stacks. Appsody offers many features which are not applicable for our "faas" context, and can be omitted for speed improvements and clarity. Additionally, breaking the dependency on a locally installed binary is of course preferable. The tentative choice of technology that has emerged is buildpacks.

We would ideally replace the appsody.Builder implementation in ./appsody/builder.go (which implements the faas.Builder interface) with a buildpacks.Builder implementation in ./buildpacks/builder.go. This could utilize the libcnd library https://github.com/buildpacks/libcnb or similar.

Other references that might help are https://buildpacks.io/docs/install-pack/ and https://github.com/lance/buildpacks

Some basic tooling should be added:

• Docker image for the final binary, rebuilt via CI.
• Releases with basic automation, rebuilt on demand.
• Verbose version information built into binary (commit hash, datestamp, semver)
• License file and headers
• (optional) Go Report card
• (optional) GoDoc reference
• (optional) Test Coverage report

The progress bar listener which is plumbed in to the create command might be a nice addition to the individual steps 'build' and 'deploy', when they are invoked directly via their subcommands. It makes no sense on init as that is generally instantaneous, and may be overkill for 'describe' and 'list' etc.

Instead of using the kn core module for Function deployment, we should be using the service client which will return the final path/route to the deployed function, so we can display this on the command line at completion.

The init step currently utilizes the appsody binary and custom stacks to initialize new Service Functions. This should be replaced by either built-in templates or a git-repo-based solution. Architecture TBD.

Replace the POC implementation of remove which wraps kubectl with the use of a k8s client.

Until it can be optimized, at least the create command needs a progress indicator for interactive terminals with verbosity disabled.

faas CLI interactive prompt shouldn't ask for Path to project directory if we are already in the directory with existing project:

$ pwd                                                                                                                                                                                                              
/Users/zroubali//boson/test2

$ ls -al                                                                                                                                                                                                           
...
-rw-r--r-- 1 zroubali 170 zář  1 16:44 .faas.yaml
..

$ faas deploy                                                                                                                                                                                                     
Path to project directory (/Users/zroubali/rh/openshift-could-functions/boson/test2):

In order to support a new domain name (TLD+1) on the Kubernetes provider, the config-domain ConfigMap currentl has to be manually updated by the cluster administrator:

apiVersion: v1
kind: ConfigMap
metadata:
  name: config-domain
  namespace: knative-serving
data:
  # Must manually add one entry per TLD+1:
  example.com: |
    selector:
      faas.domain: "example.com"
  example.org: |
    selector:
      faas.domain: "example.org"
  # Default is local only.
  svc.cluster.local: ""

This could be automated by augmenting the function Create step to alter this list.

I was also the one to introduce --trigger because I thought --template could be confusing. I often ended up typing something like faas init --template nodejs because instinctively that's how I thought about it. But I agree that --trigger could be too constraining. Maybe there's another word we can use, like --context or something (though I guess this doesn't get to the nodejs-express-webpack example you provided.)
~ @lance

Some options, with contrived complexity to hopefully get our creative juices flowing:

faas create --language nodejs --template webpack-events

faas create --language go --trigger gorilla-http

faas create --language python --context tensorflow-keras

The original version used template, which I later updated to context when it felt like that didn't encapsulate the idea of the template files plus extra contextually-available resources, but was changed back to template because of possible confusion per suggestion by @matejvasek .

If I had to choose from the above, my choice would probably still be template, mainly because (despite not being entirely accurate), it's likely to be the easiest for a new user to understand by fitting into common experiences, and directly representing the effect they are able to see directly: the form of the files written locally. But perhaps we can think of something altogether better?

https://github.com/boson-project/faas/blob/fbab8c09d05e0f08c5320c0d16b9508203e458d1/knative/deployer.go#L61-L63

It is my understanding that the label and annotation referencing the domain and subdomain are not necessary for the system to function if the configuraiton of the cluster domains is followed per the docs. This ConfigMap configuration mutation could also be automated.

Assuming the implementation is fixed per #85 , keeping these lines has the benefit that the cluster configuration is not strictly required. It has the detriment that one can no longer use kn or other non-faas-cli methods for creating a Function service with a route unless they remember to manually add these labels/annotations.

I would probably lean towards expecting that the cluster is configured correctly (I presume we will have automatin to help with this soon) rather than rely on the client to set these values. In that case, these two lines could be removed, and the check in #85 could actually be removed entirely.

Both approaches (three, really) have differing tradeoffs, so thoughts much appreciated.

We currently have the cloudevents template ("events"), and would like to add support for an http handler as well.

When running faas create <runtime> --namespace foo, the --namespace flag is referring to the image registry namespace, but there seems to be no way to declare the cluster namespace to deploy the function to. It's always faas. I wonder if it wouldn't be better to have faas create <runtime> --repository quay.io/lanceball --namespace my-cluster-namespace, consolidating the --registry and --namespace options into one --repository option, and use the --namespace flag for the cluster namespace.

Running faas --version produced v0.0.0-source.

❯ curl -L -s -o - faas.gz https://github.com/boson-project/faas/releases/download/v0.4.0/faas.gz | gunzip >faas && chmod 755 faas
❯ ./faas --version 
v0.0.0-source

We are no longer using appsody - so all of this code should probably be removed from the repo

Create documentation for getting started with a local Kind cluster. Additional notes on how to securely access the cluster (treting it as a network appliance) would be useful/

We currently us tip when tagging the container to ensure that directly pushing artifacts made during development do not become the latest default image pulled if running docker run quay.io/boson/faas. We should, however, go ahead and set a latest tag such that a user can run this command and be assured they are pulling the latest released version.

In this project the --namespace command has been used to indicate the Kubernetes namespace or alternatively the image repository namespace. Additionally, in some commands (e.g. faas deploy) the user-supplied image tag is handled as with other CLIs such as docker and podman. That is, the entire string, including the hostname, namespace (if any - it is not required) and a possible version tag.

This project should normalize these usages across all commands by always referring to a project's image as a single name (i.e. it's "tag", and the namespace as a Kubernetes namespace. Where the latter is nonsensical, for example in the deploy and update commands, the CLI help text should clearly indicate that the command is only used when managing a function project on a cluster.

Call to func overrideImage(root, override string) (err error) creates .faas.yaml. This causes that subsequent call to func (c *Client) Create(cfg Function) (err error) fails.

Some of the commands support specifying JSON output; it would be nice to have all commands support this flag.

Replace the POC implementation of deploy which wraps kubectl with the use of a k8s client.

The general architecture of the system is to provide sensible defaults for as many choice-points as possible in order to smooth the overall first-use flow. However this has the side-effect of hiding the details of what is happening, and producing sometimes inscrutable errors when the defaults are not able to be inferred. This only applies to the CLI, not the client library (which clearly deliniates the expected parameters and returns errors), so we could solve this by adding an interactive prompt when the terminal is detected to be interactive, such that defaults are clearly displayed with an option to override. This maintains the important "paved path" of default options, while exposing them for inspection and optional override.

• Prompt for choice points, such as:
  • Function name
  • Container registry
  • Skip public route (internal only)
  • Overwrite extant
• Display calculated default, allowing an [enter] to accept
• Validate user input prior to invoking the client library
• Keep the implementation separate from the client library
• Implement as a package, not hacked into cmd or main with full tests.
• Only prompt when terminal is interactive

https://github.com/boson-project/faas/blob/fbab8c09d05e0f08c5320c0d16b9508203e458d1/knative/deployer.go#L35-L42

When validating if the name is valid, we should consider the fact that 1) some DNS names have multiple levels. for example .co.uk and 2) the system supports multiple-level subdomains. for example cdn1.us-west-2.www.example.com. These conditions are supported by the CLI, by the name path-derivation mechanism, and by the KNative routing layer Kourier. Therefore updating this code to support these conditions should be the only change necessary to make it work throughout the system.

In both cases, these vagaries of domains can be addressed using the Go publicsuffix package.

One caviat: we should also consider that a common use case will be to flag a Function as "internal", meaning that it will could be suffixed with .cluster.local. We should also confirm that the publicsuffix package understands this "non-public" yet conformant suffix, supporting it manually if necessary.

This will allow folks who are not engineers to give new features and bug fixes a spin

go install of the CLI currently fails when run from outside the project directory:

$ go install github.com/boson-project/faas/cmd/faas
src/github.com/boson-project/faas/knative/deployer.go:11:2: cannot find package "knative.dev/client/pkg/kn/core" in any of:
        /usr/lib/go/src/knative.dev/client/pkg/kn/core (from $GOROOT)
        /home/drekar/go/src/knative.dev/client/pkg/kn/core (from $GOPATH)
src/github.com/boson-project/faas/knative/describer.go:9:2: cannot find package "knative.dev/eventing/pkg/apis/eventing/v1alpha1" in any of:
        /usr/lib/go/src/knative.dev/eventing/pkg/apis/eventing/v1alpha1 (from $GOROOT)
        /home/drekar/go/src/knative.dev/eventing/pkg/apis/eventing/v1alpha1 (from $GOPATH)
src/github.com/boson-project/faas/knative/describer.go:10:2: cannot find package "knative.dev/eventing/pkg/client/clientset/versioned/typed/eventing/v1alpha1" in any of:
        /usr/lib/go/src/knative.dev/eventing/pkg/client/clientset/versioned/typed/eventing/v1alpha1 (from $GOROOT)
        /home/drekar/go/src/knative.dev/eventing/pkg/client/clientset/versioned/typed/eventing/v1alpha1 (from $GOPATH

This is likely due to the knative.dev dependency therein not being based on go modules, but may be a different cause. One way to solve this may be to update the knative client dependency to the latest, which I believe are now go modules.

This does not affect standard builds from the repo root (where the go.mod and go.sum files are used

Once CI is tested and functional, promoting the credentials used to organization-level for future integrations will save future work. Currently the secrets Quay Username and Quay Token (Robot account) are created within this repository for the docker image deployment step. These should be moved to the Organization by an administrator and this repository updated to use those instead.

The shell completions are no longer fully integrated after the recent fairly sweeping update of the CLI arguments and flags in #79. This is a good time to expand them to consistently integrate in all the subcommands, updating where necessary.

Update the POC implementation of updater which wraps the kn command with proper usage of the knative client.

If I run some faas command, I get asked about overriding default namespace:

$ faas deploy                                                                                                                                                                                                      
Override default namespace (optional) (zroubalik):

The displayed value (zroubalik) is not correct, even though the project is deployed in the correct default namespace.

The displayed namespace value is probably taken from a FAAS_NAMESPACE variable that I have specified before (from my understanding this Variable is referring to image repo namespace, not k8s namespace).