-
Notifications
You must be signed in to change notification settings - Fork 265
Conversation
Update vendor for tectonic-config to introduce new cluster config types.
Update config-generator for the new tectonic-config type.
- Bump operator images. - Introduce openshift images. - Drop prometheus operator and dex.
Update bootstrap manifests to use openshift images.
Add key/cert generation for openshift apiserver, etc. Delete key/cert generation for dex.
- Introduce openshift components - Drop dex, tectonic-prometheus-operator.
- Add key/cert generation for openshift components. - Delete key/cert generation for dex.
Also introduce the kubelet bootstrap.
Can one of the admins verify this patch? |
@@ -65,7 +65,7 @@ resource "aws_security_group_rule" "master_ingress_https" { | |||
protocol = "tcp" | |||
cidr_blocks = ["0.0.0.0/0"] | |||
from_port = 6443 | |||
to_port = 6443 | |||
to_port = 6445 |
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.
what's running on 6444 and 6445?
rm /etc/kubernetes/manifests/tectonic-node-controller-pod.yaml | ||
|
||
cp -r $(pwd)/bootstrap-configs /etc/kubernetes/bootstrap-configs |
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.
why not just cp -r bootstrap-configs /etc/kubernetes/bootstrap-conf
without the $(pwd)
?
kind: Namespace | ||
metadata: | ||
# This is the namespace used to hold the openshift console. | ||
# They require openshift console run in this namespace. |
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.
Who is they
?
@@ -9,3 +9,5 @@ versions: | |||
docker: [ "17.03", "1.12"] | |||
1.9: | |||
docker: [ "17.03", "1.12"] | |||
3.10: | |||
docker: [ "17.03", "1.12"] |
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.
we should keep the newline
--volume=resolv,kind=host,source=/etc/resolv.conf \ | ||
--mount volume=resolv,target=/etc/resolv.conf \ | ||
--volume var-lib-cni,kind=host,source=/var/lib/cni \ | ||
--mount volume=var-lib-cni,target=/var/lib/cni \ | ||
--volume var-lib-kubelet,kind=host,source=/var/lib/kubelet \ | ||
--mount volume=var-lib-kubelet,target=/var/lib/kubelet \ | ||
--volume var-log,kind=host,source=/var/log \ | ||
--mount volume=var-log,target=/var/log" | ||
--mount volume=var-log,target=/var/log \ | ||
--insecure-options=image" |
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.
Why do we need to use --insecure-options
? Can we instead add a task to make the image secure?
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.
because we're pulling a docker image instead of an ACI. No choice :-(
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.
This goes away when we switch to quay.
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.
Right, this is my point. Can we either:
a: switch to quay right now
or
b: make a task to switch to quay and remove this flag
tls.key: ${ingress_tls_key} | ||
bundle.crt: ${ingress_tls_bundle} |
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 see that this commit didn't introduce the mistake but we should add a newline here.
00d0a9f
to
ebe6961
Compare
@@ -20,6 +20,7 @@ spec: | |||
containers: | |||
- name: kube-core-operator | |||
image: ${kube_core_operator_image} | |||
imagePullPolicy: Always |
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.
Is this necessary? or just some debug code that leaked?
I did a quick smoketest of this on libvirt, and it's good enough for now. There are some followup items:
But this looks good! LGTM. |
The last consumer was commented out in ebe6961 (disable out of sync tests for opentonic, 2018-06-08, coreos/tectonic-installer#3270) and that comment was removed in 31eb165 (remove unused code, 2018-06-18, coreos/tectonic-installer#3298).
No description provided.