add knative and kfserving in AWS kfdef

[theofpa]

Which issue is resolved by this Pull Request:
Resolves #1035

Description of your changes:
Add knative-serving and kfserving manifests to add the CRDs and install knative-serving and kfserving in all AWS kfdefs

Checklist:

• Unit tests have been rebuilt:
  1. cd manifests/tests
  2. make generate-changed-only
  3. make test

[kubeflow-bot]

This change is 

[k8s-ci-robot]

Hi @theofpa. Thanks for your PR.

I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[Jeffwan]

1. The process changed a little bit. You need to make the change here. https://github.com/kubeflow/manifests/tree/master/kfdef/source

Then run the command to generate manifests.

2. I have not tested secondary kubeflow-gateway, could you clarify if that's needed or not?

[Jeffwan]

Need to make the change in source folder

[karlschriek]

@theofpa, I think the parts for kfserving-gateway is missing.

  - kustomizeConfig:
      parameters:
      - name: namespace
        value: istio-system
      repoRef:
        name: manifests
        path: istio/kfserving-gateway
    name: kfserving-gateway

See #1017

[theofpa]

@theofpa, I think the parts for kfserving-gateway is missing.

  - kustomizeConfig:
      parameters:
      - name: namespace
        value: istio-system
      repoRef:
        name: manifests
        path: istio/kfserving-gateway
    name: kfserving-gateway

See #1017

I intentionally left out kfserving-gateway. It creates a classic ELB with open ports to services useful for inference and observability, which makes me uncomfortable to include, especially in the Cognito configuration.

The inferenceservices of kfserving can be consumed from workloads running inside the kubernetes cluster via the knative-serving/cluster-local-gateway. This is a service-to-service communication and indeed should not rely on Auth. In case there is a need to externally invoke those endpoints, we can have have a separate ALB with custom restriction (for example, I'm using a custom host header called x-api-key, similarly to AWS API gateway).

The exposed services for observability (kiali, grafana, etc) can be instead exposed through the main kubeflow-gateway.kubeflow, via virtualservices. These are UIs useful for users to consume, and it makes sense to have the Cognito authentication in place.

[theofpa]

Need to make the change in source folder

done in commit a071364

[Jeffwan]

Let's hold this PR for a while. Seems v1.0-branch just get updated and pinned manifest doesn't have manifest. need to be careful

[Jeffwan]

#1072 is merged.

@theofpa What's your options on kfserving-gateway? I would suggest we add it for v1.0-branch, since there's blocking issues on knative, it's fine to keep it same as other solutions because this is a temporary solution anyway. Use can still delete public endpoint if they don't like that.

Another feedback is let's revert changes to 1.0.0 and 1.0.1 file. I know the scripts auto generate changes there. But we think that's pinned to commit. So let's not touch that. Release team will create a 1.0.2 file with the change.

[Jeffwan]

/hold

[richardsliu]

We should not be updating the v1.0.0 manifests anymore. If this is a candidate for v1.0.2 then we should wait until after the new kfdef files are created and just modify those.

[theofpa]

We should not be updating the v1.0.0 manifests anymore. If this is a candidate for v1.0.2 then we should wait until after the new kfdef files are created and just modify those.

reverted in f4e0038

[theofpa]

#1072 is merged.

@theofpa What's your options on kfserving-gateway? I would suggest we add it for v1.0-branch, since there's blocking issues on knative, it's fine to keep it same as other solutions because this is a temporary solution anyway. Use can still delete public endpoint if they don't like that.

Another feedback is let's revert changes to 1.0.0 and 1.0.1 file. I know the scripts auto generate changes there. But we think that's pinned to commit. So let's not touch that. Release team will create a 1.0.2 file with the change.

added the kfserving-gateway in 8a6e0ae

The introduction of kfserving-gateway not only introduces a new gateway via a classic unsecured ELB, but also changes the gateway that knative is using, so we can no longer route the inference traffic through the kubeflow-gateway.
We need to describe the change in the e2e guide, also visually in the diagram at the bottom. In the guide, the users need to understand that this is a temp workaround until we'll introduce a proper server-to-server authentication mechanism for the inference services, as discussed in kfserving/#760

[Jeffwan]

@theofpa Thanks for the change. since v1.0.2 are already in the master. Can you help apply these changes to v1.0.2?

Steps:

1. Run command python3 hack/build_kfdef_specs.py run

2. Revert changes in kfdef/kfctl_aws_xxx_v1.0.0.yaml and kfdef/kfctl_aws_xxx_v1.0.1.yaml

[Jeffwan]

/lgtm

[Jeffwan]

/hold cancel

[richardsliu]

/retest

[Jeffwan]

/test kubeflow-manifests-presubmit

[Jeffwan]

/ok-to-test

[Jeffwan]

@theofpa Can you help rebase upstream master changes?

[richardsliu]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: richardsliu

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:

• kfdef/OWNERS [richardsliu]

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

[Jeffwan]

/lgtm