This repository has been archived by the owner on Dec 15, 2021. It is now read-only.
New CLI for triggers create/delete/update/list and new IT tests #630
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andresmgot
reviewed
Mar 14, 2018
| limitations under the License. | ||
| */ | ||
|
|
||
| package http |
There was a problem hiding this comment.
shouldn't we merge the trigger http with the route command?
cmd/kubeless/trigger/http/list.go
Outdated
| var listCmd = &cobra.Command{ | ||
| Use: "list FLAG", | ||
| Aliases: []string{"ls"}, | ||
| Short: "list all Cronjob triggers deployed to Kubeless", |
| ns = utils.GetDefaultNamespace() | ||
| } | ||
|
|
||
| functionName, err := cmd.Flags().GetString("function-name") |
There was a problem hiding this comment.
suggestion: --function already implies the name
| } | ||
|
|
||
| func doList(w io.Writer, kubelessClient versioned.Interface, ns string) error { | ||
| triggersList, err := kubelessClient.KubelessV1beta1().CronJobTriggers(ns).List(metav1.ListOptions{}) |
There was a problem hiding this comment.
you need to filter the cronjobs for the ones with created-by=kubeless
cmd/kubeless/trigger/http/create.go
Outdated
| Run: func(cmd *cobra.Command, args []string) { | ||
|
|
||
| if len(args) != 1 { | ||
| logrus.Fatal("Need exactly one argument - cronjob trigger name") |
cmd/kubeless/trigger/http/create.go
Outdated
| Ports: []v1.ServicePort{ | ||
| { | ||
| Name: "http-function-port", | ||
| NodePort: 0, |
There was a problem hiding this comment.
what this NodePort: 0 means?
cmd/kubeless/trigger/http/update.go
Outdated
| if port != 0 { | ||
| httpTrigger.Spec.ServiceSpec.Ports[0].Port = port | ||
| httpTrigger.Spec.ServiceSpec.Ports[0].TargetPort = intstr.FromInt(int(port)) | ||
| } |
There was a problem hiding this comment.
you can move this duplicated code to a common function
pkg/utils/k8sutil.go
Outdated
| @@ -151,6 +151,20 @@ func GetHTTPTriggerClientOutCluster() (versioned.Interface, error) { | |||
| return kubelessClient, nil | |||
| } | |||
|
|
|||
| // GetKubelessClientOutCluster returns kubeless clientset for making requests from outside of cluster | |||
| func GetKubelessClientOutCluster() (versioned.Interface, error) { | |||
There was a problem hiding this comment.
there is no difference between this function and the other Get*ClientOutCluster, why don't we merge them?
murali-reddy
force-pushed
the
function-trigger-runtimes
branch
5 times, most recently
from
de9e4e9 to
358f8cb
Compare
* Avoid to write in ConfigMap volume * Adapt test
murali-reddy
force-pushed
the
triggers-cmd
branch
from
f3abb5b to
0099ea5
Compare
3 tasks
* CRD API objects for functions, http, kafka, cronjob triggers and corresponding auto-generated informer, listers, clienset and deep copy generated function * CRD controllers for the function, kafka triggers, cronjob triggers, http triggers CRD objects. * new server side binaries kubeless-controller-manager and kafka-controller * updated CLI to incorporate implict creation of triggers with "kubeless function deploy" and move ingress object creation logic to http trigger controller * updated to build scripts * updated to manifests Co-authored-by: Andres <andres.mgotor@gmail.com> Co-authored-by: Tuna <ng.tuna@gmail.com> * Updates to examples used in integration tests Co-authored-by: Andres <andres.mgotor@gmail.com> Co-authored-by: Tuna <ng.tuna@gmail.com> * kafka even consumer with new go kafka lib dependencies Co-authored-by: Tuna <ng.tuna@gmail.com> * update vendor libs Co-authored-by: Tuna <ng.tuna@gmail.com> * update docker files Co-authored-by: Tuna <ng.tuna@gmail.com> Co-authored-by: Andres <andres.mgotor@gmail.com> * updates to utils and langruntime * update .travis.yaml * delete old controller docker file * better kafka even consumer logging * fix travis ci issue related to kafka broker * keep the old GKE version 1.7.12-gke.1 * Use bitnami-bot docker account * Fix http request method for the Kafka consumer * Push Kafka image * Fix content-type in consumer HTTP message * Fix GKE version * Removed duplicated files * Update runtime docs * Do proper check for topic update * skip IT test with trigger update through * Add CPU limit argument to function commands (vmware-archive#606) * Add CPU limit argument to function commands * Fix doc typo * Add better description to --cpu argument * fix glide.lock and vendor libs, that got messed up with master merge * addressing review comments * update .gitignore with new controller images * Review * Review * use 3-way merge Patch only to update the CRD objects * nodejs, remove runtime internal data from sandbox, avoid code concat (vmware-archive#633) * nodejs, remove runtime internal data from sandbox, avoid code concat * update according to comments * keep compatibility with module.exports * adjust kubeless.jsonnet * Move images to the kubeless repository * Revert "use 3-way merge Patch only to update the CRD objects" This reverts commit 358f8cb. * Avoid reflect.DeepEqual for deployment comparison * Fix empty body for NodeJS functions * fix GKE CI failures due to upstream issue kubernetes/kubernetes#60538 * Fix error catching in Python runtime. Remove old files * Send message as jsons if necessary in kafka-consumer * Move serviceSpec to function object. Fix service modifications
* Path Definition for Ingress Enabled HTTP Triggers * Path Definition for Ingress Enabled HTTP Triggers * Path Definition for Ingress Enabled HTTP Triggers
murali-reddy
force-pushed
the
triggers-cmd
branch
from
0099ea5 to
1012ef8
Compare
|
Closing this PR in favour of #643 opened against master. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue Ref: [Issue number related to this PR or None]
Adding new CLI and integratio tests. Will mark for review once IT tests are stabilized in the CI.
Description:
[PR Description]
TODOs: