Kafka integration test #922
Merged
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
pavannd1
approved these changes
Mar 25, 2021
PrasadG193
reviewed
Mar 30, 2021
pkg/app/kafka.go
Outdated
Comment on lines
97
to
99
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Don't need this condition, we are anyways returning err on line no 100
pkg/app/kafka.go
Outdated
| "kafka-ping", | ||
| "-ti", | ||
| "--rm=true", | ||
| "--image=strimzi/kafka:0.20.0-kafka-2.6.0", |
There was a problem hiding this comment.
let's declare the image name as const
pkg/app/kafka.go
Outdated
| "--", | ||
| "bin/kafka-topics.sh", | ||
| "--list", | ||
| "--bootstrap-server=my-cluster-kafka-external-bootstrap:9094", |
There was a problem hiding this comment.
is the bootstrap server name depends on the release name we provide to install chart? If that is the case, we should make it variable and build the address from release name.
Please also declare port as const.
There was a problem hiding this comment.
here bootstrap server doesn't depend on the release name. I will add the hostname and port as constant.
pkg/app/kafka.go
Outdated
Comment on lines
303
to
312
| config, err := rest.InClusterConfig() | ||
| if err == nil { | ||
| return config, nil | ||
| } | ||
|
|
||
| homeConfig := filepath.Join(os.Getenv("HOME"), ".kube/config") | ||
| config, err = clientcmd.BuildConfigFromFlags("", homeConfig) | ||
| if err == nil { | ||
| return config, nil | ||
| } |
There was a problem hiding this comment.
Please use kube.NewClient() method in pkg/kube to init K8s client
https://github.com/kanisterio/kanister/blob/master/pkg/kube/client.go#L54
pkg/app/kafka.go
Outdated
Comment on lines
322
to
330
| cfg, err := LoadConfig() | ||
| if err != nil { | ||
| return nil, errors.Wrapf(err, "Failed to Load config") | ||
| } | ||
|
|
||
| clientset, err := kubernetes.NewForConfig(cfg) | ||
| if err != nil { | ||
| return nil, errors.Wrapf(err, "Failed to create Clienset for k8s config") | ||
| } |
There was a problem hiding this comment.
pkg/app/kafka.go
Outdated
| return nil, errors.Wrapf(err, "Failed to get service for component") | ||
| } | ||
| for k, v := range svc.Spec.Selector { | ||
| selector = fmt.Sprintf("%s%s=%s,", selector, k, v) |
There was a problem hiding this comment.
| conn, err := kafka.DialLeader(ctx, "tcp", uri, topic, partition) | ||
| if err != nil { | ||
| return 0, err | ||
| } |
There was a problem hiding this comment.
Add defer conn.Close() and remove the call on Line no. 440
Co-authored-by: Prasad Ghangal <prasad.ghangal@gmail.com>
PrasadG193
approved these changes
Mar 30, 2021
Co-authored-by: Prasad Ghangal <prasad.ghangal@gmail.com>
PrasadG193
added a commit
that referenced
this pull request
Apr 5, 2021
* Kafka Backup Blueprint * added newline * echo on single line * fixed typo * moved to examples folder * Adding kafka Restore blueprint using confluent source connector * properties file for s3Source * changes to restore blueprint * addition to restore blueprint * resolved issues * add error exit command * files renamed * added prehook * restore all topics, added a python script * merged blueprints * added phase to restore action * successcode * pod success * backup Process completed with successful exit code * fixed bugs * restore with input artifact * fixed bug * customized connector * backup and restore done * adobe s3 connector * fixed newline at the end * changes to README, multiple Run commands in Dockerfile * required changes in goreleaser * bug fixes * removed latest tags * adding comments * source connector name as pod name * removed jar file, added delete action, removed cleanup process, change kafka cluster setup * added sink docker image to delete action * removed clusterRole dependency, changed kafdrop broker address * typo correction * changes to add delete action * latest changes * Added Install functionality for kafka * completed install, ready and Ping test * WIP to delete blueprint * added delete action * lint fixed * lint checked * resolved dependencies * updated the test to use helm * typo fix * added var for yaml path * minor changes * review comments * removed kubernetes var * removing errors.Wrapf * delete CRD and configmap * blueprint images * corrected file path * removed cluster and role bindings in CRD deletion * delete kafka cluster * wait on deployment ready * deleted crd delete functionality * removed uninstalling CRD * conflict resolved * blueprint image changed * image to ghcr * Apply suggestions from code review * Fix typo * Apply suggestions from code review Co-authored-by: Prasad Ghangal <prasad.ghangal@gmail.com> * Applied suggestions from review comments * Update pkg/app/kafka.go Co-authored-by: Prasad Ghangal <prasad.ghangal@gmail.com> * minor change Co-authored-by: Pavan Navarathna <pavan@kasten.io> Co-authored-by: Prasad Ghangal <prasad.ghangal@gmail.com>
chaitanya-baraskar
pushed a commit
to chaitanya-baraskar/kanister
that referenced
this pull request
Apr 5, 2021
* Kafka Backup Blueprint * added newline * echo on single line * fixed typo * moved to examples folder * Adding kafka Restore blueprint using confluent source connector * properties file for s3Source * changes to restore blueprint * addition to restore blueprint * resolved issues * add error exit command * files renamed * added prehook * restore all topics, added a python script * merged blueprints * added phase to restore action * successcode * pod success * backup Process completed with successful exit code * fixed bugs * restore with input artifact * fixed bug * customized connector * backup and restore done * adobe s3 connector * fixed newline at the end * changes to README, multiple Run commands in Dockerfile * required changes in goreleaser * bug fixes * removed latest tags * adding comments * source connector name as pod name * removed jar file, added delete action, removed cleanup process, change kafka cluster setup * added sink docker image to delete action * removed clusterRole dependency, changed kafdrop broker address * typo correction * changes to add delete action * latest changes * Added Install functionality for kafka * completed install, ready and Ping test * WIP to delete blueprint * added delete action * lint fixed * lint checked * resolved dependencies * updated the test to use helm * typo fix * added var for yaml path * minor changes * review comments * removed kubernetes var * removing errors.Wrapf * delete CRD and configmap * blueprint images * corrected file path * removed cluster and role bindings in CRD deletion * delete kafka cluster * wait on deployment ready * deleted crd delete functionality * removed uninstalling CRD * conflict resolved * blueprint image changed * image to ghcr * Apply suggestions from code review * Fix typo * Apply suggestions from code review Co-authored-by: Prasad Ghangal <prasad.ghangal@gmail.com> * Applied suggestions from review comments * Update pkg/app/kafka.go Co-authored-by: Prasad Ghangal <prasad.ghangal@gmail.com> * minor change Co-authored-by: Pavan Navarathna <pavan@kasten.io> Co-authored-by: Prasad Ghangal <prasad.ghangal@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Change Overview
Adding integration test for Kafka Blueprint. This will not be tested in kanister CI, but we will be using it in k10 pipeline.
Link to Log statement : https://gist.github.com/A-kanksh-a/a84f0d7df5b3c2f4e953b1728267d93c#file-kafka-integration-test-log
Pull request type
Please check the type of change your PR introduces:
Issues
Test Plan
./build/integration-test.sh Kafka