BUG 1670700: *: add support for etcd metrics proxy #517

hexfusion · 2019-03-01T18:49:24Z

This PR adds support for new TLS assets to support the etcd metrics proxy. This proxy will create a separate chain of trust for metrics consumer which will not give allow access to server data directly. The proxy itself is a feature of etcd called the grpc-proxy. By exposing a separate metrics port on the server and then using the proxy to listen to that port we eliminate direct gRPC transactions.

The grpc-proxy forwards only /metrics endpoint requests to the server.

/cc @brancz @abhinavdahiya

depends on: openshift/installer#1291 merged

runcom · 2019-03-01T18:55:11Z

@hexfusion could you please also add a target for these PRs (4.0 or 4.1 or next)? We're after freeze and if this is a feature I'd love someone like Clayton or Derek or Eric or others to approve

runcom · 2019-03-01T18:55:19Z

/hold

kikisdeliveryservice · 2019-03-01T18:56:57Z

Is this for 4.0 or 4.1?

brancz · 2019-03-06T10:22:01Z

As mentioned in openshift/installer#1291, this needs to land in 4.0.

runcom · 2019-03-06T10:35:11Z

e2es are failing with starting kube though, could you take a look?

/retest

hexfusion · 2019-03-06T13:03:22Z

working through this now

hexfusion · 2019-03-06T13:33:40Z

I am building

/var/opt/openshift/tls/etcd-metrics-ca-bundle.crt
/var/opt/openshift/tls/etcd-metrics-signer-client.crt
/var/opt/openshift/tls/etcd-metrics-signer-client.key
/var/opt/openshift/tls/etcd-metrics-signer-server.crt
/var/opt/openshift/tls/etcd-metrics-signer-server.key

But bootstrap wants different so back to the installer.

Mar 06 13:32:06 ip-10-0-5-133 bootkube.sh[29781]: F0306 13:32:06.133573       1 bootstrap.go:111] error rendering bootstrap manifests: open /etc/ssl/etcd/metrics-ca.crt: no such file or directory

hexfusion · 2019-03-06T19:37:08Z

/test e2e-aws

hexfusion · 2019-03-06T20:16:39Z

We have a little bit of a chicken and egg issue here :) as bootstrap depends on itself from the installer.

abhinavdahiya · 2019-03-06T21:29:44Z

pkg/apis/machineconfiguration.openshift.io/v1/types.go

+	RootCAData        []byte `json:"rootCAData"`
+
+	// Metrics
+	EtcdMetricsServerCertData []byte `json:"etcdMetricsServerCertData"`


I would like to use objectrefs for secret object.

runcom · 2019-03-06T22:35:00Z

cmd/machine-config-operator/bootstrap.go

 	}
 )

 func init() {
 	rootCmd.AddCommand(bootstrapCmd)
 	bootstrapCmd.PersistentFlags().StringVar(&bootstrapOpts.etcdCAFile, "etcd-ca", "/etc/ssl/etcd/ca.crt", "path to etcd CA certificate")
+	bootstrapCmd.PersistentFlags().StringVar(&bootstrapOpts.etcdMetricsCAFile, "etcd-metrics-ca", "", "path to etcd metrics CA certificate")
+	bootstrapCmd.PersistentFlags().StringVar(&bootstrapOpts.etcdMetricsServerCertFile, "etcd-metrics-server-crt", "", "path to etcd metrics server certificate")
+	bootstrapCmd.PersistentFlags().StringVar(&bootstrapOpts.etcdMetricsServerKeyFile, "etcd-metrics-server-key", "", "path to etcd metrics server key")


AIUI, these flags should provide a dummy/default value for this PR to not depend on the installer. After this goes in, you can land the installer PR and then create another PR here to remove the dummy/default value to always honor what bootkube gives you.

(this is not changing or adding images from the release payload so it should be an easier dance)

/cc @cgwalters

ok sounds good that was my thought as well.

brancz · 2019-03-07T09:54:01Z

I can see the metrics client cert in the e2e test artifacts 🎉 now just need to satisfy CI I guess :)

hexfusion · 2019-04-01T20:03:23Z

@abhinavdahiya PTAL

/cc @brancz

kikisdeliveryservice · 2019-04-01T20:11:25Z

@hexfusion does this have/need an exception granted?

cc: @runcom

hexfusion · 2019-04-01T20:19:18Z

@hexfusion does this have/need an exception granted?

Yeah @kikisdeliveryservice monitoring will chime in, probably tomorrow.

s-urbaniak · 2019-04-02T06:56:55Z

/approve
... if the invariants expressed in openshift/installer#1483 (review) still hold.

Note that I do not have the authority to grant exceptions though! This must be done higher level.

runcom · 2019-04-02T08:15:00Z

cc @smarterclayton

s-urbaniak · 2019-04-02T12:37:42Z

xref openshift/cluster-monitoring-operator#302

Signed-off-by: Sam Batschelet <sbatsche@redhat.com>

hexfusion · 2019-04-03T12:55:50Z

We have a confirmation on this exception from @derekwaynecarr.

/cc @runcom

runcom · 2019-04-03T13:47:21Z

just saw that, thanks

/lgtm

openshift-ci-robot · 2019-04-03T13:47:30Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hexfusion, runcom, s-urbaniak

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~~ [runcom]

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

hexfusion · 2019-04-03T14:23:49Z

/hold cancel

s-urbaniak · 2019-04-03T14:32:20Z

@runcom @hexfusion awesome, thanks for the hard work ... now let's see if etcd appears in monitoring! 🎉

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 Mar 1, 2019

openshift-ci-robot requested review from abhinavdahiya and brancz March 1, 2019 18:49

openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 1, 2019

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

hexfusion mentioned this pull request Mar 1, 2019

pkg/asset/tls: add etcd metrics assets openshift/installer#1291

Merged

runcom added the 4.0 label Mar 6, 2019

hexfusion force-pushed the add_proxy branch 3 times, most recently from 935cc96 to d308d79 Compare March 6, 2019 17:34

hexfusion mentioned this pull request Mar 6, 2019

data/data/bootstrap: add etcd metrics flags openshift/installer#1377

Closed

hexfusion changed the title ~~[WIP] *: add support for etcd metrics proxy and deploy TLS assets.~~ *: add support for etcd metrics proxy and deploy TLS assets. Mar 6, 2019

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 Mar 6, 2019

abhinavdahiya reviewed Mar 6, 2019

View reviewed changes

openshift-ci-robot requested a review from cgwalters March 6, 2019 22:35

runcom reviewed Mar 6, 2019

View reviewed changes

hexfusion force-pushed the add_proxy branch from 91f6bf7 to c373065 Compare March 7, 2019 01:02

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 7, 2019

hexfusion force-pushed the add_proxy branch from c373065 to 56ba051 Compare March 7, 2019 01:13

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 7, 2019

openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 1, 2019

hexfusion force-pushed the add_proxy branch from a8d9553 to a18f8d2 Compare April 1, 2019 18:19

s-urbaniak mentioned this pull request Apr 2, 2019

pkg/manifests/operator: synthesize etcd secret openshift/cluster-monitoring-operator#302

Merged

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 2, 2019

hexfusion force-pushed the add_proxy branch from a18f8d2 to 6f25f7d Compare April 3, 2019 11:31

openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 3, 2019

*: add support for etcd metric proxy

f1aaf4f

Signed-off-by: Sam Batschelet <sbatsche@redhat.com>

hexfusion force-pushed the add_proxy branch from 6f25f7d to f1aaf4f Compare April 3, 2019 11:32

openshift-ci-robot requested a review from runcom April 3, 2019 12:55

hexfusion changed the title *: add support for etcd metrics proxy BUG 1670700: *: add support for etcd metrics proxy Apr 3, 2019

openshift-ci-robot assigned runcom Apr 3, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 3, 2019

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 3, 2019

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

openshift-merge-robot merged commit c1ba218 into openshift:master Apr 3, 2019

hexfusion deleted the add_proxy branch April 3, 2019 14:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG 1670700: *: add support for etcd metrics proxy #517

BUG 1670700: *: add support for etcd metrics proxy #517

hexfusion commented Mar 1, 2019 •

edited

Loading

runcom commented Mar 1, 2019 •

edited

Loading

runcom commented Mar 1, 2019

kikisdeliveryservice commented Mar 1, 2019

brancz commented Mar 6, 2019

runcom commented Mar 6, 2019

hexfusion commented Mar 6, 2019

hexfusion commented Mar 6, 2019

hexfusion commented Mar 6, 2019

hexfusion commented Mar 6, 2019

abhinavdahiya Mar 6, 2019

runcom Mar 6, 2019 •

edited

Loading

hexfusion Mar 7, 2019

brancz commented Mar 7, 2019

hexfusion commented Apr 1, 2019

kikisdeliveryservice commented Apr 1, 2019

hexfusion commented Apr 1, 2019

s-urbaniak commented Apr 2, 2019

runcom commented Apr 2, 2019

s-urbaniak commented Apr 2, 2019

hexfusion commented Apr 3, 2019

runcom commented Apr 3, 2019

openshift-ci-robot commented Apr 3, 2019

hexfusion commented Apr 3, 2019

s-urbaniak commented Apr 3, 2019

BUG 1670700: *: add support for etcd metrics proxy #517

BUG 1670700: *: add support for etcd metrics proxy #517

Conversation

hexfusion commented Mar 1, 2019 • edited Loading

runcom commented Mar 1, 2019 • edited Loading

runcom commented Mar 1, 2019

kikisdeliveryservice commented Mar 1, 2019

brancz commented Mar 6, 2019

runcom commented Mar 6, 2019

hexfusion commented Mar 6, 2019

hexfusion commented Mar 6, 2019

hexfusion commented Mar 6, 2019

hexfusion commented Mar 6, 2019

abhinavdahiya Mar 6, 2019

Choose a reason for hiding this comment

runcom Mar 6, 2019 • edited Loading

Choose a reason for hiding this comment

hexfusion Mar 7, 2019

Choose a reason for hiding this comment

brancz commented Mar 7, 2019

hexfusion commented Apr 1, 2019

kikisdeliveryservice commented Apr 1, 2019

hexfusion commented Apr 1, 2019

s-urbaniak commented Apr 2, 2019

runcom commented Apr 2, 2019

s-urbaniak commented Apr 2, 2019

hexfusion commented Apr 3, 2019

runcom commented Apr 3, 2019

openshift-ci-robot commented Apr 3, 2019

hexfusion commented Apr 3, 2019

s-urbaniak commented Apr 3, 2019

hexfusion commented Mar 1, 2019 •

edited

Loading

runcom commented Mar 1, 2019 •

edited

Loading

runcom Mar 6, 2019 •

edited

Loading