-
Notifications
You must be signed in to change notification settings - Fork 63
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
(knative, kfserving, cert-manager) Build working solution for kfserving integration #212
Merged
google-oss-robot
merged 4 commits into
GoogleCloudPlatform:master
from
zijianjoy:match-upstream-knative22
Apr 21, 2021
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
acb28dc
(knative, kfserving, cert-manager) Build working solution for kfservi…
zijianjoy 2d7c27d
(knative) kustomize knative using expanded manifest and namespace patch
zijianjoy 84e9722
(cert-manager) Adjust cert-manager and kubeflow-issuer for v1.3
zijianjoy a586585
Merge branch 'master' into match-upstream-knative22
zijianjoy File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
16 changes: 16 additions & 0 deletions
16
kubeflow/common/cert-manager/cert-manager-1-3/cert-manager/base/kustomization.yaml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
apiVersion: kustomize.config.k8s.io/v1beta1 | ||
kind: Kustomization | ||
|
||
resources: | ||
- https://github.com/jetstack/cert-manager/releases/download/v1.3.0/cert-manager.yaml | ||
|
||
images: | ||
- name: quay.io/jetstack/cert-manager-controller | ||
newName: quay.io/jetstack/cert-manager-controller | ||
newTag: v1.3.0 | ||
- name: quay.io/jetstack/cert-manager-cainjector | ||
newName: quay.io/jetstack/cert-manager-cainjector | ||
newTag: v1.3.0 | ||
- name: quay.io/jetstack/cert-manager-webhook | ||
newName: quay.io/jetstack/cert-manager-webhook | ||
newTag: v1.3.0 |
6 changes: 6 additions & 0 deletions
6
...low/common/cert-manager/cert-manager-1-3/cert-manager/kubeflow-issuer/cluster-issuer.yaml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
apiVersion: cert-manager.io/v1alpha2 | ||
kind: ClusterIssuer | ||
metadata: | ||
name: kubeflow-self-signing-issuer | ||
spec: | ||
selfSigned: {} |
10 changes: 10 additions & 0 deletions
10
...flow/common/cert-manager/cert-manager-1-3/cert-manager/kubeflow-issuer/kustomization.yaml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
# Define the self-signed issuer for Kubeflow | ||
apiVersion: kustomize.config.k8s.io/v1beta1 | ||
kind: Kustomization | ||
namespace: cert-manager | ||
commonLabels: | ||
kustomize.component: cert-manager | ||
app.kubernetes.io/component: cert-manager | ||
app.kubernetes.io/name: cert-manager | ||
resources: | ||
- cluster-issuer.yaml |
11 changes: 11 additions & 0 deletions
11
...ommon/cert-manager/cert-manager-1-3/cert-manager/overlays/letsencrypt/cluster-issuer.yaml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
apiVersion: cert-manager.io/v1alpha2 | ||
kind: ClusterIssuer | ||
metadata: | ||
name: letsencrypt-prod | ||
spec: | ||
acme: | ||
email: $(acmeEmail) | ||
http01: {} | ||
privateKeySecretRef: | ||
name: letsencrypt-prod-secret | ||
server: $(acmeUrl) |
35 changes: 35 additions & 0 deletions
35
...common/cert-manager/cert-manager-1-3/cert-manager/overlays/letsencrypt/kustomization.yaml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
apiVersion: kustomize.config.k8s.io/v1beta1 | ||
kind: Kustomization | ||
bases: | ||
- ../../base | ||
namespace: cert-manager | ||
resources: | ||
- cluster-issuer.yaml | ||
commonLabels: | ||
kustomize.component: cert-manager | ||
app.kubernetes.io/component: cert-manager | ||
app.kubernetes.io/name: cert-manager | ||
configMapGenerator: | ||
- name: cert-manager-parameters | ||
behavior: merge | ||
envs: | ||
- params.env | ||
generatorOptions: | ||
disableNameSuffixHash: true | ||
vars: | ||
- name: acmeEmail | ||
objref: | ||
kind: ConfigMap | ||
name: cert-manager-parameters | ||
apiVersion: v1 | ||
fieldref: | ||
fieldpath: data.acmeEmail | ||
- name: acmeUrl | ||
objref: | ||
kind: ConfigMap | ||
name: cert-manager-parameters | ||
apiVersion: v1 | ||
fieldref: | ||
fieldpath: data.acmeUrl | ||
configurations: | ||
- params.yaml |
2 changes: 2 additions & 0 deletions
2
kubeflow/common/cert-manager/cert-manager-1-3/cert-manager/overlays/letsencrypt/params.env
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
acmeEmail= | ||
acmeUrl=https://acme-v02.api.letsencrypt.org/directory |
5 changes: 5 additions & 0 deletions
5
kubeflow/common/cert-manager/cert-manager-1-3/cert-manager/overlays/letsencrypt/params.yaml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
varReference: | ||
- path: spec/acme/email | ||
kind: ClusterIssuer | ||
- path: spec/acme/server | ||
kind: ClusterIssuer |
13 changes: 13 additions & 0 deletions
13
...common/cert-manager/cert-manager-1-3/cert-manager/overlays/self-signed/kustomization.yaml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
# TODO(https://github.com/kubeflow/manifests/issues/1052) clean up | ||
# the manifests after the refactor is done. We should move | ||
# cluster-issuer into the kubeflow-issuer package. | ||
apiVersion: kustomize.config.k8s.io/v1beta1 | ||
kind: Kustomization | ||
bases: | ||
- ../../base | ||
resources: | ||
- ../../kubeflow-issuer | ||
commonLabels: | ||
kustomize.component: cert-manager | ||
app.kubernetes.io/component: cert-manager | ||
app.kubernetes.io/name: cert-manager |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
apiVersion: kustomize.config.k8s.io/v1beta1 | ||
kind: Kustomization | ||
resources: | ||
- ./application # {"$kpt-set":"cert-manager-cert-manager-v3"} | ||
- ./upstream/cert-manager-crds/base | ||
- ./upstream/cert-manager-kube-system-resources/base # {"$kpt-set":"cert-manager-cert-manager-kube-system-resources-base"} | ||
# - ./application # {"$kpt-set":"cert-manager-cert-manager-v3"} | ||
# - ./upstream/cert-manager-crds/base | ||
# - ./upstream/cert-manager-kube-system-resources/base # {"$kpt-set":"cert-manager-cert-manager-kube-system-resources-base"} | ||
- ./cert-manager-1-3/cert-manager/base |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we create an issue to log this problem with a bit more details?
kubernetes-sigs/kustomize#1251
My understanding is that there are duplicate resources between several kustomize folders. Are we including them correctly? If yes, maybe we should file an upstream issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are 2 issues that I came across with regards to the knative manifests. The first is a bug in kustomize causing YAML anchors to not work properly. The second is that the
serving-core.yaml
contains all of the CRDs that are in theserving-crds.yaml
file. As such, you can't use both when using Kustomize, but you might run into a chicken & egg so you need to apply the manifests twice if your not using something that continuously syncs like Argo CD.I expanded the manifests to remove the YAML anchors and created the following kustomize folder for the Argo CD installation: argoflow/argoflow@ce115f0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you Yuan and David for the suggestion! Indeed if I include both
serving-crd.yaml
andserving-core.yaml
, it will fail with duplicated definition. However, I encountered another issue when using onlyserving-core.yaml
. I created an issue in #217.For the
manifest expand
which David has made, do you think it makes sense to upload thisexpanded serving-core.yaml
tokubeflow/manifests
repo? Otherwise every distribution needs to perform such change to a knative source file.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe a decision was made to not update KNative (or cert-manager for that matter) in the manifests repo before the 1.3 release. So I believe the route forward is to add these manifests in the gcp-blueprints repo directly for now, and try and get the fixes upstreamed after the release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for clarifying, let's merge the fix in
kubeflow/manifests
after the release. We will do the patch for now on gcp-blueprints.