Skip to content

Conversation

@grantr
Copy link
Contributor

@grantr grantr commented May 15, 2019

Now that 0.6 has been released, we can update all the install links to v0.6.0.

@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label May 15, 2019
@knative-prow-robot knative-prow-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label May 15, 2019
@grantr grantr force-pushed the update-release-versions branch from 0d8f753 to e2ea3db Compare May 15, 2019 23:01
@knative-prow-robot knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 15, 2019
Copy link
Contributor

@sebgoa sebgoa left a comment

Choose a reason for hiding this comment

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

lgtm but just a quick question about istio 1.0.7. Minikube/minishift and docker desktop install point to istio 1.0.7. Is that the correct version with 0.6 ?


```shell
curl -L https://raw.githubusercontent.com/knative/serving/v0.5.2/third_party/istio-1.0.7/istio.yaml \
curl -L https://raw.githubusercontent.com/knative/serving/v0.6.0/third_party/istio-1.0.7/istio.yaml \
Copy link
Contributor

Choose a reason for hiding this comment

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

Still Istio 1.0.7 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think @grantr made this PR before #1314 was merged in, which removed the references to our provided Istio files in 0.6. The new instructions from @ZhiminXiang now point users to download Istio directly from their website, and they instruct folks to download Istio 1.1.3: https://github.com/knative/docs/pull/1314/files#diff-1116bcf887cf6b5e21e5e4f96fa026b1

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, actually, I was going to save updating this file for later, since the Istio install here is doing something different, and I'm not sure it's possible to do this LoadBalancer/NodePort switch when installing Istio through helm?

We shouldn't update the reference here to 0.6, because we did not release an Istio.yaml for 0.6. We should leave in the reference to 0.5.2 for now, until these instructions can be updated. I created an issue to track the work -- #1319

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@samodell would you like me to revert all changes to the Minikube, Minishift, Openshift, and Docker for Mac files, or just the istio URLs?

Choose a reason for hiding this comment

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

Ah, actually, I was going to save updating this file for later, since the Istio install here is doing something different, and I'm not sure it's possible to do this LoadBalancer/NodePort switch when installing Istio through helm?

@samodell It is possible. you can configure helm flag gateways.istio-ingressgateway.type to choose the gateway service type. See https://istio.io/docs/reference/config/installation-options/#gateways-options

```shell
kubectl apply --filename https://raw.githubusercontent.com/knative/serving/v0.5.2/third_party/istio-1.0.7/istio-crds.yaml &&
curl -L https://raw.githubusercontent.com/knative/serving/v0.5.2/third_party/istio-1.0.7/istio.yaml \
kubectl apply --filename https://raw.githubusercontent.com/knative/serving/v0.6.0/third_party/istio-1.0.7/istio-crds.yaml &&
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, 1.0.7 ?

@samodell
Copy link
Contributor

samodell commented May 17, 2019

Hey @grantr -- if you're still up for it, it would be great to revert all the changes to the minkube/minishift/openshift/etc. files. I think that's better than mismatching V0.6 with outdated Istio instructions, and then we can handle getting those instructions correctly updated soon.

@grantr grantr force-pushed the update-release-versions branch from 308f7a6 to dec11fc Compare May 17, 2019 20:40
@grantr
Copy link
Contributor Author

grantr commented May 17, 2019

if you're still up for it, it would be great to revert all the changes to the minkube/minishift/openshift/etc. files.

No problem! I rebased on top of the latest master and reverted the changes to the Minikube, Minishift, OpenShift, and Docker for Mac install instructions.

@samodell
Copy link
Contributor

/lgtm
/approve

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label May 17, 2019
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: grantr, samodell

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow-robot knative-prow-robot merged commit 7638203 into knative:master May 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants