Skip to content
This repository has been archived by the owner on Aug 2, 2019. It is now read-only.

WIP: Get the demo working on OCP4 #30

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

WIP: Get the demo working on OCP4 #30

wants to merge 8 commits into from

Conversation

jcrossley3
Copy link
Contributor

Update registry name and IP for curl command. Still lots of minishift references to remove

Update registry name and IP for curl command. Still lots of minishift
references to remove
@@ -268,7 +268,8 @@ servicemesh, which is listening on `$(minishift ip):32380`). Stringed together w
command:

```bash
$ curl -H "Host: dumpy.myproject.example.com" "http://$(minishift ip):32380/health"
$ IP=$(oc get -n istio-system service istio-ingressgateway -ojsonpath='{.status.loadBalancer.ingress[0].hostname}')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we guaranteed to have a hostname set here for every OCP4 environment? Usually scripts branch between either ip or hostname (whatever is set).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know, actually. On the only supported platform (AWS), I expect so. But I agree we can probably do this bettererer

Copy link
Contributor

Choose a reason for hiding this comment

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

We should do better! We should get real DNS here and not a host header.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Easy enough on a shared cloud. I hate to ignore the local cluster peeps, though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, this issue is about OCP4, so we can assume AWS somewhere for now perhaps?

Copy link
Contributor

Choose a reason for hiding this comment

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

An OpenShift Route with dumpy.myproject.<my ocp cluster domain> and an edit to Knative's config-network to change the domain from example.com to <my ocp cluster domain>?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this might be fine getting in as is. We need to find the sequence of commands to make things externally available on OCP4 anyway, we can probably augment that in a subsequent PR if we want to.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

* Scale down revision in 60s instead of the default 5m
@bbrowning
Copy link
Contributor

Do we still need this PR? Or do we want to forever keep ocp4 support on a separate branch?

@jcrossley3
Copy link
Contributor Author

I assumed we would eventually merge it. Once we remove references to minishift and verify it works on some supported OCP4 platform, e.g. AWS

bbrowning and others added 5 commits March 11, 2019 08:55
This prevents smaller OpenShift 4 clusters from running out of CPU
just to run this demo. The Knative default is 400m and this cuts that
down to 1/8th, or 50m. These pods don't consume any real CPU anyway.
Using productized image and its event type
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants