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 doc in response to changes to install #618

Closed
wants to merge 7 commits into from

Conversation

kalantar
Copy link
Member

@kalantar kalantar commented Apr 14, 2021

Addresses issue #615

@kalantar kalantar marked this pull request as draft April 14, 2021 16:53
Copy link
Member

@sriumcp sriumcp left a comment

Choose a reason for hiding this comment

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

Are there additional changes needed for automated and manual test files?

@@ -19,7 +19,7 @@ You will create the following resources in this tutorial.
- eventually replaces `baseline` with `candidate` using Kustomize

???+ warning "Before you begin, you will need... "
**Kubernetes cluster:** Ensure that you have Kubernetes cluster with Iter8 and Knative installed. You can do this by following Steps 1, 2, and 3 of the [quick start tutorial for Knative](../../../getting-started/quick-start/with-knative/).
**Kubernetes cluster:** Ensure that you have a Kubernetes cluster with Iter8, Knative and the sample metrics for Knative (which assume Prometheus) installed. You can do this by following Steps 1, 2, and 3 of the [quick start tutorial for Knative](../../../getting-started/quick-start/with-knative/).
Copy link
Member

Choose a reason for hiding this comment

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

Ensure that you have a Kubernetes cluster with Iter8, Knative, Iter8 sample metrics for Knative, and the Iter8 Prometheus add-on installed.

## 1. Create versions
## 1. Give Permission to Iter8 to Call `helm upgrade`

The final step of the experiment is to promote the winning version. In this sample experiment, this is done using `helm upgrade`. Helm uses secrets to record information about an installation. Iter8 must have permission to these resources.
Copy link
Member

Choose a reason for hiding this comment

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

Helm uses secrets to record information about releases. This tutorial uses an experiment that invokes Helm. Enable this experiment using the following RBAC.

@@ -30,6 +30,7 @@ if [[ ! " ${NETWORK_LAYERS[@]} " =~ " ${1} " ]]; then
fi

# Step 1: Export correct tags for install artifacts
export GIT_ACCOUNT="${GIT_ACCOUNT:iter8-tools}"
Copy link
Member

Choose a reason for hiding this comment

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

Why is there a minus in lines 34 and 35 but none in 33?

Copy link
Member Author

Choose a reason for hiding this comment

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

my mistake. There should be a "-"; see https://stackoverflow.com/a/48829326/3482067

@kalantar
Copy link
Member Author

@sriumcp thanks for initial comments; now addressed. A question I have is whether the way metrics are installed -- in platformsetup.sh (https://github.com/kalantar/iter8/blob/install-changes/samples/knative/quickstart/platformsetup.sh#L123) is appropriate vs an explicit step in the quickstart. I see little value in an explicit step.

@sriumcp
Copy link
Member

sriumcp commented Apr 14, 2021

Some thoughts on metrics...

A/B and A/B/n experiments are all about business metrics, or in ML serving use-cases, ML specific metrics. In A/B, A/B/n experiments with business/ML metrics, we will want to show how custom (business/ML) Iter8 metrics are created, installed and used as part of the experiment.

For Canary/Conformance experiments, perhaps we can simply say, when you did the "easy install" of Iter8 as part of quick start, sample metrics were installed along with Prometheus add-on; we will use them in this experiment.

Our A/B, and A/B/n experiments so far have involved a combination of business + system metrics (latency/error-rate). So, even in A/B and A/B/n, some metrics are available as part of easy install, and others need to be shown as part of the tutorial.

No metrics == no experiments... so, the metrics story is just as integral to each tutorial as the experiment story (although, so far we haven't made an effort to say that story).

@kalantar
Copy link
Member Author

No metrics == no experiments... so, the metrics story is just as integral to each tutorial as the experiment story (although, so far we haven't made an effort to say that story).

For Istio, I tested Istio A/B/n example with productpage with a business reward. We can include this metric in the tutorial as an initial place to tell this story.

@kalantar kalantar marked this pull request as ready for review April 14, 2021 21:27
sriumcp and others added 5 commits April 18, 2021 18:33
@kalantar
Copy link
Member Author

Not sure why the merge introduced so many file changes. Closing and recreating to focus on the relevant changes.

@kalantar kalantar closed this Apr 19, 2021
@kalantar kalantar mentioned this pull request Apr 19, 2021
@kalantar kalantar deleted the install-changes branch May 4, 2021 13:15
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.

2 participants