-
Notifications
You must be signed in to change notification settings - Fork 55
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
Update k8s installer for refactored contiv components #317
Conversation
Dropped codes for k8s1.4, updated scripts and yaml files for contiv services. Made a branch based install testing for k8s, and changed k8s gating process. Also update etcd to v3.2.4, it still behaves as etcd2, because contiv connects it with v2 api, just need to use newer container to make it having sh in it. Signed-off-by: Wei Tie <wtie@cisco.com>
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.
lightly reviewed
install/ansible/install.sh
Outdated
cluster_store_type=consul | ||
cluster_store_urls=$OPTARG | ||
install_etcd=false | ||
;; |
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 think this is a tab vs spaces thing
install/ansible/install.sh
Outdated
cluster_store="etcd://localhost:2379" | ||
if [ "$cluster_store" = "" ]; then | ||
cluster_store_type="etcd" | ||
cluster_store_urls="http://localhost:2379" |
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.
random spacing 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.
i'll submit a new patch to format the codes
effect: NoSchedule | ||
initContainers: | ||
- name: contiv-etcd-init | ||
image: ferest/etcd-initer:latest |
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 should be pinned or even rebuilt and in our hub?
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 was wrong with just using args for the etcd container?
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.
explained at README of https://hub.docker.com/r/ferest/etcd-initer/
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 is currently not pined ( i guess won't be updated anyway like the sidecar project), I didn't make it in contiv dockerhub was because by that time i wasn't able to link it with contiv dockerhub. we can revisit it
scripts/prepare_netplugin_images.sh
Outdated
if [ -z "${NETPLUGIN_BRANCH:-}" ]; then | ||
echo "Trying to use dockerhub contiv/netplugin:${CONTIV_NETPLUGIN_VERSION}" | ||
# ensure the image exists | ||
http_rc=$(curl -s -w "%{http_code}" -o /dev/null https://hub.docker.com/v2/repositories/contiv/netplugin/tags/${CONTIV_NETPLUGIN_VERSION}/) |
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 pulls the image, try using HEAD instead of GET with curl
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 pulls meta data of the image, not downloading it
Signed-off-by: Wei Tie <wtie@cisco.com>
install/k8s/configs/contiv.yaml
Outdated
readOnly: false | ||
- mountPath: /etc/kubernetes/ssl | ||
name: etc-kubernetes-ssl | ||
readOnly: false |
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.
make readonly
scripts/kubeadm_test.sh
Outdated
vagrant ssh-config $vm > $keyfile | ||
for img in $(ls -1 ../${install_version}/contiv_cache/*.tar); do | ||
scp -F $keyfile $img $vm:/tmp/ | ||
ssh -F $keyfile $vm -- sudo docker load -i /tmp/$(basename $img) |
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.
maybe cat tar to ssh tar extract to reduce bytes . . not a blocker
scripts/prepare_netplugin_images.sh
Outdated
if [ -z "${NETPLUGIN_BRANCH:-}" ]; then | ||
echo "Trying to use dockerhub contiv/netplugin:${CONTIV_NETPLUGIN_VERSION}" | ||
# ensure the image exists | ||
http_rc=$(curl -s -w "%{http_code}" -o /dev/null https://hub.docker.com/v2/repositories/contiv/netplugin/tags/${CONTIV_NETPLUGIN_VERSION}/) |
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.
add curl -L
scripts/prepare_netplugin_images.sh
Outdated
if [ "$http_rc" = 200 ]; then | ||
echo "Found contiv/netplugin:${CONTIV_NETPLUGIN_VERSION} on dockerhub, exit 0" | ||
exit 0 | ||
elif [ "$http_rc" = 404 ]; then |
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.
handle else case
Signed-off-by: Wei Tie <wtie@cisco.com>
scripts/prepare_netplugin_images.sh
Outdated
# tempdir for building and cleanup on exit | ||
netplugin_tmp_dir="$(mktemp -d)" | ||
trap 'rm -rf ${netplugin_tmp_dir}' EXIT | ||
else |
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.
the if part exits in all cases, else block isn't needed, not a blocker
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.
build PR
@@ -0,0 +1,77 @@ | |||
#!/bin/bash |
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 feels wrong. Can you explain why this approach is needed?
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 need a way to build images that not on dockerhub
build PR |
Signed-off-by: Wei Tie <wtie@cisco.com>
build PR |
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.
seems OK
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.
thanks for the walkthrough earlier
Dropped codes for k8s1.4, updated scripts and yaml files for
contiv services. Made a branch based install testing for k8s,
and changed k8s gating process.
Also update etcd to v3.2.4, it still behaves as etcd2, because contiv
connects it with v2 api, just need to use newer container to make it
having sh in it.
Signed-off-by: Wei Tie wtie@cisco.com