Cleanup oadm router somewhat #7251

smarterclayton · 2016-02-12T06:23:49Z

Still more work to do in the router command, it's gotten... opaque.

smarterclayton · 2016-02-12T07:14:27Z

@detiber starting with 1.2/3.2 we will want to pass --subdomain=ROUTER_SUBDOMAIN to oadm router in a single le. The default config should remain for back compat.

smarterclayton · 2016-02-12T07:16:27Z

[test]

smarterclayton · 2016-02-12T22:56:35Z

Ok, this now includes even more function - it:

remove hostport support from oadm router - it's useless and someone can set that on their own
store default certificate as a secret and plumb the path all the way down to the haproxy instance
change the default from using ENV for secrets to using service account, and generate resources for each thing that needs to be set
- default certificate is passed as bytes if this flag is set, otherwise is set as a path environment variable
cleanup the SCC warning on the router
the --ports mapping is now repurposed to the mapping on the router service (to make routers easier to remap)
support --subdomain as a template

I've done local testing and everything seems to be ok, but needs a bit more look and love.

@detiber FYI this is changing router default to create a service account - all other flags are the same, but you can now omit --credentials, and we probably want to start setting --subdomain

@pweil- @ramr for review

liggitt · 2016-02-15T15:39:05Z

pkg/cmd/admin/router/router.go

@@ -64,20 +66,31 @@ you have failover protection.`
 	secretsVolumeName = "secret-volume"
 	secretsPath       = "/etc/secret-volume"

+	defaultCertificateDir = "/etc/pki/tls/private"


is this standard? if so, [citation needed] as a comment

It's the RH standard, vs ubuntu which is using the older /etc/ssl

smarterclayton · 2016-02-16T16:46:33Z

@ramr review please

smarterclayton · 2016-02-17T04:14:04Z

moar eyes

pweil- · 2016-02-17T14:58:52Z

@Kargakis - would you mind giving this some eyeballs since I'm tied up still?

0xmichalis · 2016-02-17T16:51:38Z

pkg/cmd/admin/router/router.go

+			ObjectMeta: kapi.ObjectMeta{
+				Name: fmt.Sprintf("%s-certs", cfg.Name),
+			},
+			Type: "kubernetes.io/tls",


https://github.com/kubernetes/kubernetes/blob/0866c377e9a84a73e0ff78efdd1def7f7e8b437a/pkg/api/types.go#L2311

Not in our branch yet

On Wed, Feb 17, 2016 at 11:51 AM, Michail Kargakis <[email protected]

wrote:

In pkg/cmd/admin/router/router.go
#7251 (comment):

Name: secretsVolumeName, ReadOnly: true, MountPath: secretsPath, }

mounts = append(mounts, mount)

}

if len(defaultCert) > 0 {

// TODO: extract the private key from the CRT and set it as its own key

// TODO: use upstream constants for this

secret := &kapi.Secret{

ObjectMeta: kapi.ObjectMeta{

Name: fmt.Sprintf("%s-certs", cfg.Name),

},

Type: "kubernetes.io/tls",

https://github.com/kubernetes/kubernetes/blob/0866c377e9a84a73e0ff78efdd1def7f7e8b437a/pkg/api/types.go#L2311

—
Reply to this email directly or view it on GitHub
https://github.com/openshift/origin/pull/7251/files#r53193982.

smarterclayton · 2016-02-18T19:02:35Z

@liggitt review last commit

liggitt · 2016-02-18T22:09:58Z

one question on newline in encoding PEM blocks

smarterclayton · 2016-02-18T22:50:01Z

Does add trailing newline.

Don't require user to have SCC access to create a router, clean up errors, check the appropriate scc permissions. Stop exposing hostPorts and use secrets for the default certificate.

smarterclayton · 2016-02-18T22:50:26Z

[merge]

openshift-bot · 2016-02-18T23:00:42Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/5028/) (Image: devenv-rhel7_3476)

openshift-bot · 2016-02-18T23:00:42Z

Evaluated for origin merge up to 241d02b

smarterclayton · 2016-02-19T00:38:52Z

[test]

smarterclayton · 2016-02-19T04:12:45Z

Well this is a nice flake:

ERROR: gave up waiting for https://127.0.0.1:38443/healthz

!!! Error in /data/src/github.com/openshift/origin/hack/../hack/util.sh:370
    'return 1' exited with status 1
Call stack:
    1: /data/src/github.com/openshift/origin/hack/../hack/util.sh:370 start_os_master(...)
    2: /data/src/github.com/openshift/origin/hack/update-generated-swagger-spec.sh:50 main(...)
Exiting with status 1
[INFO] Dumping container logs to /tmp/openshift/generate-swagger-spec//logs
[INFO] Dumping all resources to /tmp/openshift/generate-swagger-spec//logs/export_all.json
Authentication required for https://127.0.0.1:38443 (openshift)
Username: system:admin
Password: error: username system:admin is invalid for basic auth
error: failed to negotiate an api version; server supports: map[], client supports: map[v1:{} v1beta3:{} extensions/v1beta1:{} metrics/v1alpha1:{} componentconfig/v1alpha1:{} authorization.k8s.io/v1beta1:{}]

smarterclayton · 2016-02-19T04:25:11Z

[test]

smarterclayton · 2016-02-19T06:38:43Z

Error from server: buildconfigs "test" not found
W0219 00:44:52.316886    5128 cmd.go:212] -p POD is DEPRECATED and will be removed in a future version. Use exec POD instead.
!!! Error in hack/../hack/../test/end-to-end/core.sh:299
    'grep -q "unable to validate against any security context constraint"' exited with status 0 1
Call stack:
    1: hack/../hack/../test/end-to-end/core.sh:299 main(...)
Exiting with status 1
Error from server: User "e2e-default-admin" cannot list all buildconfigs in the cluster

[test]

openshift-bot · 2016-02-19T06:45:27Z

Evaluated for origin test up to 241d02b

openshift-bot · 2016-02-19T08:15:22Z

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/1380/)

Merged by openshift-bot

openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 12, 2016

smarterclayton force-pushed the cleanup_plugins branch 2 times, most recently from fc91672 to 1231ab0 Compare February 12, 2016 22:51

smarterclayton added the component/routing label Feb 12, 2016

openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 12, 2016

smarterclayton force-pushed the cleanup_plugins branch 2 times, most recently from b6ed377 to f4302bc Compare February 13, 2016 06:16

liggitt reviewed Feb 15, 2016
View reviewed changes

smarterclayton force-pushed the cleanup_plugins branch from f4302bc to ec6bd91 Compare February 15, 2016 17:07

0xmichalis reviewed Feb 17, 2016
View reviewed changes

Get rid of the plugins/ dir

a398a60

smarterclayton force-pushed the cleanup_plugins branch from ffc743f to 0bdd6e9 Compare February 18, 2016 15:21

smarterclayton force-pushed the cleanup_plugins branch from 8f44d9f to f2d7d02 Compare February 18, 2016 20:48

Allow subdomain flag to create router

241d02b

Don't require user to have SCC access to create a router, clean up errors, check the appropriate scc permissions. Stop exposing hostPorts and use secrets for the default certificate.

smarterclayton force-pushed the cleanup_plugins branch from f2d7d02 to 241d02b Compare February 18, 2016 22:50

openshift-bot pushed a commit that referenced this pull request Feb 19, 2016

Merge pull request #7251 from smarterclayton/cleanup_plugins

dd651fa

Merged by openshift-bot

openshift-bot merged commit dd651fa into openshift:master Feb 19, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup oadm router somewhat #7251

Cleanup oadm router somewhat #7251

smarterclayton commented Feb 12, 2016

smarterclayton commented Feb 12, 2016

smarterclayton commented Feb 12, 2016

smarterclayton commented Feb 12, 2016

liggitt Feb 15, 2016

smarterclayton Feb 15, 2016 via email

smarterclayton commented Feb 16, 2016

smarterclayton commented Feb 17, 2016

pweil- commented Feb 17, 2016

0xmichalis Feb 17, 2016

smarterclayton Feb 17, 2016

smarterclayton commented Feb 18, 2016

liggitt commented Feb 18, 2016

smarterclayton commented Feb 18, 2016

smarterclayton commented Feb 18, 2016

openshift-bot commented Feb 18, 2016

openshift-bot commented Feb 18, 2016

smarterclayton commented Feb 19, 2016

smarterclayton commented Feb 19, 2016

smarterclayton commented Feb 19, 2016

smarterclayton commented Feb 19, 2016

openshift-bot commented Feb 19, 2016

openshift-bot commented Feb 19, 2016

Cleanup oadm router somewhat #7251

Cleanup oadm router somewhat #7251

Conversation

smarterclayton commented Feb 12, 2016

smarterclayton commented Feb 12, 2016

smarterclayton commented Feb 12, 2016

smarterclayton commented Feb 12, 2016

liggitt Feb 15, 2016

Choose a reason for hiding this comment

smarterclayton Feb 15, 2016 via email

Choose a reason for hiding this comment

smarterclayton commented Feb 16, 2016

smarterclayton commented Feb 17, 2016

pweil- commented Feb 17, 2016

0xmichalis Feb 17, 2016

Choose a reason for hiding this comment

smarterclayton Feb 17, 2016

Choose a reason for hiding this comment

smarterclayton commented Feb 18, 2016

liggitt commented Feb 18, 2016

smarterclayton commented Feb 18, 2016

smarterclayton commented Feb 18, 2016

openshift-bot commented Feb 18, 2016

openshift-bot commented Feb 18, 2016

smarterclayton commented Feb 19, 2016

smarterclayton commented Feb 19, 2016

smarterclayton commented Feb 19, 2016

smarterclayton commented Feb 19, 2016

openshift-bot commented Feb 19, 2016

openshift-bot commented Feb 19, 2016