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

Update flink-demo to use operator dependencies #279

Merged
merged 2 commits into from
Aug 6, 2020

Conversation

zen-dog
Copy link
Contributor

@zen-dog zen-dog commented Jun 25, 2020

Summary:

  • using KudoOperator task to include Zookeeper, Kafka and Flink dependencies directly instead of pre-installing them manually (and activate using Instance resources)
  • the corresponding parameters are in the params files e.g. templates/kafka-params.yaml
  • README is updated to reflect the latest changes
  • demo relies on not yet releases kudoctl and manager

Signed-off-by: Aleksey Dukhovniy alex.dukhovniy@googlemail.com

Summary:
- using `KudoOperator` task to include Zookeeper, Kafka and Flink dependencies directly instead of pre-installing them manually (and activate using `Instance` resources)
- the corresponding parameters are in the params files e.g. `templates/kafka-params.yaml`
- README is updated to reflect the latest changes
- demo relies on not yet releases kudoctl and manager

Signed-off-by: Aleksey Dukhovniy <alex.dukhovniy@googlemail.com>
@zen-dog zen-dog requested review from zmalik, nfnt and guenter June 25, 2020 12:11
zen-dog added a commit to kudobuilder/kudo that referenced this pull request Jun 25, 2020
Summary:
after updating the [flink-demo](kudobuilder/operators#279) to use dependencies a few fixes were needed:
- instance controller now properly reconciles (parent) instances owning child instances
- `InClusterResolver` ignores operators `appVersion` if it wasn't set
- child instance names are generated differently:
  - if the `instanceName` was set, it is taken without modifications
  - otherwise, `parentInstance-childOperator` is taken e.g. `kafka-zookeeper`. in this case we also use `-` instead of `.` separator since instance names are often used as service names (DNS-1035 label) which only allow alphanumeric characters or `-` in them.

Signed-off-by: Aleksey Dukhovniy <alex.dukhovniy@googlemail.com>
zen-dog pushed a commit to kudobuilder/kudo that referenced this pull request Jun 29, 2020
Summary:
after updating the [flink-demo](kudobuilder/operators#279) to use dependencies a few fixes were needed:
- instance controller now properly reconciles (parent) instances owning child instances
- `InClusterResolver` ignores operators `appVersion` if it wasn't set
- child instance names are generated differently:
  - if the `instanceName` was set, it is taken without modifications
  - otherwise, `parentInstance-childOperator` is taken e.g. `kafka-zookeeper`. in this case we also use `-` instead of `.` separator since instance names are often used as service names (DNS-1035 label) which only allow alphanumeric characters or `-` in them.

Signed-off-by: Aleksey Dukhovniy <alex.dukhovniy@googlemail.com>
Copy link
Member

@kensipe kensipe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My tests needed to deviate because things are not released... details below (but I believe this should work)... the end result issue:

kubectl kudo plan status --instance flink-demo
Plan(s) for "flink-demo" in namespace "default":
.
└── flink-demo (Operator-Version: "flink-demo-0.1.5" Active-Plan: "deploy")
    └── Plan deploy (serial strategy) [FATAL_ERROR], last updated 2020-07-01 08:37:22
        ├── Phase dependencies (serial strategy) [FATAL_ERROR]
        │   ├── Step zookeeper [FATAL_ERROR] (default/flink-demo fatal error:  failed to build task deploy.dependencies.zookeeper.zookeeper: task validation error: kudo operator task 'zookeeper' has an empty operator name)

Deviations:

  1. running local manager with KUDO_CERT_DIR=./test/cert/ make run
  2. Installing from the local files system: go run cmd/kubectl-kudo/main.go install ../operators/repository/flink/docs/demo/financial-fraud/demo-operator/ --instance flink-demo

repository/flink/docs/demo/financial-fraud/README.md Outdated Show resolved Hide resolved
Signed-off-by: Aleksey Dukhovniy <alex.dukhovniy@googlemail.com>
Copy link
Member

@zmalik zmalik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@zmalik
Copy link
Member

zmalik commented Jul 9, 2020

and tested it with 0.15.0-rc1

kubectl kudo plan status --instance=flink-demo
Plan(s) for "flink-demo" in namespace "default":
.
└── flink-demo (Operator-Version: "flink-demo-0.1.5" Active-Plan: "deploy")
    └── Plan deploy (serial strategy) [COMPLETE], last updated 2020-07-09 11:32:20
        ├── Phase dependencies (serial strategy) [COMPLETE]
        │   ├── Step zookeeper [COMPLETE]
        │   └── Step kafka [COMPLETE]
        ├── Phase flink-cluster (serial strategy) [COMPLETE]
        │   └── Step flink [COMPLETE]
        ├── Phase demo (serial strategy) [COMPLETE]
        │   ├── Step gen [COMPLETE]
        │   └── Step act [COMPLETE]
        └── Phase flink-job (serial strategy) [COMPLETE]
            └── Step submit [COMPLETE]

Copy link
Member

@kensipe kensipe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good!

@zen-dog zen-dog merged commit faa87ce into master Aug 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants