Use MachineConfigOperator instead of TNCO #232

abhinavdahiya · 2018-09-11T00:41:10Z

Requires https://github.com/coreos-inc/tectonic-operators/pull/456 and openshift/machine-config-operator#50

/hold

crawford · 2018-09-11T20:47:08Z

/lgtm

wking · 2018-09-11T20:48:51Z

installer/pkg/config-generator/tls.go

-	// TNC certs
-	tncDomain := fmt.Sprintf("%s-tnc.%s", c.Name, c.BaseDomain)
+	// MachineConfigServer certs
+	mcsDomain := fmt.Sprintf("%s-tnc.%s", c.Name, c.BaseDomain)


Do we want to update the template to %s-mcs.%s or similar? And you probably want to make similar changes to pkg/asset/tls.

https://jira.coreos.com/browse/CORS-833 is going to consolidate the api endpoints. So left it for now.

yifan-gu · 2018-09-11T20:53:11Z

lgtm to me, same question as @wking pointed out.

abhinavdahiya · 2018-09-11T20:53:42Z

/hold

wking · 2018-09-11T20:56:24Z

modules/bootkube/resources/bootkube.sh

+# /etc/ssl/mcs/tls.{crt, key} are locations for MachineConfigServer's tls assets.
+cp "$PWD/tls/machine-config-server.crt" /etc/ssl/mcs/tls.crt
+cp "$PWD/tls/machine-config-server.key" /etc/ssl/mcs/tls.key
+cp "$PWD/mco-bootstrap/machineconfigoperator-bootstrap-pod.yaml" /etc/kubernetes/manifests/


This doesn't appear to be a local file. Is it created by the bootstrap rendering call above? I think this would be less brittle if we were able to volume-mount a handful of target directories, and then the installer could put its generated assets in the right places on its own. But if this is the only such file, than handling it explicitly here is probably an acceptable short-term solution.

We need special logic for some operators that we need for bootstrapping the cluster:

installer/modules/bootkube/resources/bootkube.sh

Lines 16 to 27 in 77cc46f

# shellcheck disable=SC2154

/usr/bin/docker run \

--user 0 \

--volume "$PWD:/assets:z" \

"${tnc_operator_image}" \

--config=/assets/tnco-config.yaml \

--render-bootstrap=true \

--render-output=/assets/tnc-bootstrap

mkdir -p /etc/kubernetes/manifests/

cp "$PWD/tnc-bootstrap/tectonic-node-controller-pod.yaml" /etc/kubernetes/manifests/

cp "$PWD/tnc-bootstrap/tectonic-node-controller-config.yaml" /etc/kubernetes/tnc-config

installer/modules/bootkube/resources/bootkube.sh

Line 92 in 77cc46f

cp -r "$PWD/bootstrap-configs" /etc/kubernetes/bootstrap-configs

Everything else is already present in the correct directory <something>/manifests

Is it created by the bootstrap rendering call above?

yes

rphillips · 2018-09-11T21:15:54Z

Are we bringing over the checkpointer in this PR as well?

abhinavdahiya · 2018-09-11T21:20:17Z

@rphillips checkpointer is installed by kube-core-operator ??

rphillips · 2018-09-11T21:25:18Z

looks like it is in kube-core-operator. thanks!

wking · 2018-09-11T22:55:59Z

installer/pkg/config-generator/generator.go

@@ -13,7 +13,6 @@ import (
 	"github.com/coreos/tectonic-config/config/kube-addon"
 	"github.com/coreos/tectonic-config/config/kube-core"
 	"github.com/coreos/tectonic-config/config/tectonic-network"
-	tnco "github.com/coreos/tectonic-config/config/tectonic-node-controller"


You probably want a separate commit rebuilding BUILD.bazel and cleaning this our of vendor/.

I used https://github.com/openshift/installer#go to create df62984. I didn't remove glide.lock, as i didn't want to update the whole world :/

I didn't remove glide.lock, as i didn't want to update the whole world :/

Removing glide.lock wasn't too bad when I did it in 2018-08-30 in 2e835b0. But I'm fine punting the vendor/ cleanup down the road if you want.

abhinavdahiya · 2018-09-12T03:17:21Z

requires openshift/machine-config-operator#54
#222 changed how the installconfig was rendered.

wking · 2018-09-12T03:59:39Z

I landed openshift/machine-config-operator#54. I expect that will have bumped the v4.0.0 tag you're using in this PR automatically, so I'll just kick the tests again:

/retest

crawford · 2018-09-12T15:55:06Z

/lgtm

openshift-ci-robot · 2018-09-12T15:55:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, crawford

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:

~~OWNERS~~ [abhinavdahiya,crawford]

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

abhinavdahiya · 2018-09-12T16:04:28Z

/hold cancel

openshift-ci-robot · 2018-09-12T16:12:35Z

@abhinavdahiya: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/prow/e2e-aws	`df62984`	link	`/test e2e-aws`
ci/prow/e2e-aws-smoke	`df62984`	link	`/test e2e-aws-smoke`
ci/prow/build-tarball	`df62984`	link	`/test build-tarball`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

… cluster-api Generated with: $ glide update --strip-vendor $ glide-vc --use-lock-file --no-tests --only-code $ bazel run //:gazelle using: $ glide --version (cd $GOPATH/src/github.com/Masterminds/glide && git describe) v0.13.1-7-g3e13fd1 $ (cd $GOPATH/src/github.com/sgotti/glide-vc && git describe) v0.1.0-2-g6ddf6ee $ bazel version Build label: 0.16.1- (@non-git) Build target: bazel-out/k8-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar Build time: Mon Aug 13 16:42:29 2018 (1534178549) Build timestamp: 1534178549 Build timestamp as int: 1534178549 The tectonic-node-controller removal catches us up with 596591b (.*: replace tectonic node controller with machine config operator, 2018-09-10, openshift#232). The cluster-api trim adjusts the content from b00e40e (vendor: Add client from sigs.k8s.io/cluster-api, 2018-09-04, openshift#119). Because cluster-api wasn't in glide.lock, I suspect neither glide nor glide-vc were run before that commit.

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 11, 2018

openshift-ci-robot requested review from aaronlevy and wking September 11, 2018 00:41

openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 11, 2018

abhinavdahiya force-pushed the tnco_to_mco branch 3 times, most recently from 163ebc4 to 9518ab5 Compare September 11, 2018 19:17

abhinavdahiya changed the title ~~WIP: Use MachineConfigOperator instead of TNCO~~ Use MachineConfigOperator instead of TNCO Sep 11, 2018

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 11, 2018

openshift-ci-robot assigned crawford Sep 11, 2018

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 11, 2018

wking reviewed Sep 11, 2018

View reviewed changes

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 11, 2018

wking reviewed Sep 11, 2018

View reviewed changes

abhinavdahiya force-pushed the tnco_to_mco branch from 9518ab5 to 4fc3971 Compare September 11, 2018 21:02

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Sep 11, 2018

abhinavdahiya force-pushed the tnco_to_mco branch from 4fc3971 to 96d2e00 Compare September 11, 2018 22:40

wking reviewed Sep 11, 2018

View reviewed changes

abhinavdahiya added 2 commits September 11, 2018 18:28

.*: replace tectonic node controller with machine config operator

596591b

*: update BUILD.bazel

df62984

abhinavdahiya force-pushed the tnco_to_mco branch from 96d2e00 to df62984 Compare September 12, 2018 01:29

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 12, 2018

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 12, 2018

openshift-merge-robot merged commit 84ab99e into openshift:master Sep 12, 2018

abhinavdahiya deleted the tnco_to_mco branch September 12, 2018 16:28

abhinavdahiya mentioned this pull request Sep 18, 2018

modules/bootkube: update operatorstatus to namespaced #272

Merged

wking mentioned this pull request Sep 20, 2018

vendor: Glide update, droppping tectonic-node-controller and trimming cluster-api #291

Closed

abhinavdahiya mentioned this pull request Sep 20, 2018

*: clean up AWS ELBs #242

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use MachineConfigOperator instead of TNCO #232

Use MachineConfigOperator instead of TNCO #232

abhinavdahiya commented Sep 11, 2018 •

edited

Loading

crawford commented Sep 11, 2018

wking Sep 11, 2018

yifan-gu Sep 11, 2018

abhinavdahiya Sep 11, 2018

yifan-gu commented Sep 11, 2018

abhinavdahiya commented Sep 11, 2018

wking Sep 11, 2018

abhinavdahiya Sep 11, 2018

abhinavdahiya Sep 11, 2018

rphillips commented Sep 11, 2018

abhinavdahiya commented Sep 11, 2018

rphillips commented Sep 11, 2018

wking Sep 11, 2018

abhinavdahiya Sep 12, 2018

wking Sep 12, 2018

abhinavdahiya commented Sep 12, 2018

wking commented Sep 12, 2018

crawford commented Sep 12, 2018

openshift-ci-robot commented Sep 12, 2018

abhinavdahiya commented Sep 12, 2018

openshift-ci-robot commented Sep 12, 2018

	# shellcheck disable=SC2154
	/usr/bin/docker run \
	--user 0 \
	--volume "$PWD:/assets:z" \
	"${tnc_operator_image}" \
	--config=/assets/tnco-config.yaml \
	--render-bootstrap=true \
	--render-output=/assets/tnc-bootstrap

	mkdir -p /etc/kubernetes/manifests/
	cp "$PWD/tnc-bootstrap/tectonic-node-controller-pod.yaml" /etc/kubernetes/manifests/
	cp "$PWD/tnc-bootstrap/tectonic-node-controller-config.yaml" /etc/kubernetes/tnc-config

Use MachineConfigOperator instead of TNCO #232

Use MachineConfigOperator instead of TNCO #232

Conversation

abhinavdahiya commented Sep 11, 2018 • edited Loading

crawford commented Sep 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yifan-gu commented Sep 11, 2018

abhinavdahiya commented Sep 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rphillips commented Sep 11, 2018

abhinavdahiya commented Sep 11, 2018

rphillips commented Sep 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abhinavdahiya commented Sep 12, 2018

wking commented Sep 12, 2018

crawford commented Sep 12, 2018

openshift-ci-robot commented Sep 12, 2018

abhinavdahiya commented Sep 12, 2018

openshift-ci-robot commented Sep 12, 2018

abhinavdahiya commented Sep 11, 2018 •

edited

Loading