rename cluster-osin-operator/cluster-authentication-operator #17

sallyom · 2019-01-11T14:36:16Z

@enj @evb changed the import paths here, too
cluster-osin-operator to cluster-authentication-operator
and using the naming conventions laid out by @deads2k in email thread:

ClusterOperator name  "authentication" - see note below
Operator Namespace “openshift-authentication-operator”   - this PR
Operator Deployment “cluster-authentication-operator”  - this PR
Operator Git repo “openshift/cluster-authentication-operator”  - will be changed
Operand Namespace “openshift-authentication” - this PR

*note: - in this PR it's "cluster-authentication-operator". There isn't consistency with that
name in clusters today, although going with majoritiy it would be "openshift-authentication-operator"

ericavonb · 2019-01-15T20:09:00Z

@sallyom want to rebase? let's get this thing merged!

enj · 2019-01-15T21:12:36Z

pkg/operator2/operator.go

 func defaultLabels() map[string]string {
 	return map[string]string{
-		"app": "origin-cluster-osin-operator2",
+		"app": "origin-cluster-authentication-operator2",


we don't need the 2

done, fixed

enj · 2019-01-15T21:13:23Z

pkg/operator2/operator.go


 const (
-	targetName = "openshift-osin" // TODO fix
+	targetName = "openshift-integrated-auth"


can you outline the new name requirements so we know if this matches

from email thread (kube-apiserver example), naming conventions from @deads2k are as follows:

ClusterOperator name "authentication" Operator Namespace “openshift-authentication-operator” Operator Deployment “cluster-authentication-operator” Operator Git repo “openshift/cluster-authentication-operator” Operand Namespace “openshift-authentication” -edited targetName above to match

enj · 2019-01-15T21:14:38Z

pkg/cmd/operator2/cmd.go

+	componentNamespace = "openshift-authentication-operator"
 )

 func NewOperator() *cobra.Command {


cmd.User needs fixing

I've changed to operator

enj · 2019-01-15T21:14:44Z

pkg/cmd/operator2/cmd.go

 const (
-	componentName      = "cluster-osin-operator2"
-	componentNamespace = "openshift-core-operators"
+	componentName      = "cluster-authentication-operator2"


enj · 2019-01-15T21:14:55Z

pkg/cmd/operator2/cmd.go

-	componentName      = "cluster-osin-operator2"
-	componentNamespace = "openshift-core-operators"
+	componentName      = "cluster-authentication-operator2"
+	componentNamespace = "openshift-authentication-operator"


cite requirements

from email thread:

ClusterOperator name "authentication" Operator Deployment “cluster-authentication-operator” Operator Namespace “openshift-authentication-operator”

so, componentName should be "authentication" above according to email? However, looking at a cluster, most operators have the name "openshift-something-operator" and a few have "cluster-something-operator". I'm keeping with cluster-authentication-operator unless I hear otherwise, keeping with minimal change here.

enj · 2019-01-15T21:15:21Z

pkg/cmd/operator/cmd.go

 const (
-	componentName      = "cluster-osin-operator"
-	componentNamespace = "openshift-core-operators"
+	componentName      = "cluster-authentication-operator"


do not touch anything in pkg/operator

enj · 2019-01-15T21:16:39Z

manifests/0000_09_cluster-authentication-operator_05_deploy.yaml

 metadata:
-  namespace: openshift-core-operators
-  name: origin-cluster-osin-operator
+  namespace: openshift-authentication-operator


do not mess with the original operator, we have no tests

enj · 2019-01-15T21:16:49Z

manifests/0000_09_cluster-authentication-operator_05_deploy.yaml

        image: quay.io/openshift/origin-cluster-authentication-operator:latest
        imagePullPolicy: IfNotPresent
-        command: ["osin-operator", "operator"]
+        command: ["authentication-operator", "operator"]


this is the only line I expect to change

so for both deployments, should be same, right?
command: ["authentication-operator", "operator"] (also in pkg/cmd/operator|operator2/cmd.go, same cmd.User for both? operator?)

enj · 2019-01-15T21:17:07Z

manifests/0000_09_cluster-authentication-operator_05_deploy.yaml

 metadata:
-  namespace: openshift-core-operators
-  name: origin-cluster-osin-operator2
+  namespace: openshift-authentication-operator


lots of 2's that need to die

removed them

enj · 2019-01-15T21:19:55Z

I do not think the risk of breaking operator1 is worth it, so I do no think we should touch the old operator at all. At the bare minimum openshift/origin#21733 needs to be merged before this.

ericavonb

/lgtm

sallyom · 2019-01-16T19:51:06Z

/hold cancel

sallyom · 2019-01-16T23:28:14Z

/hold
I'll rebase after other open PRs merge/unhold after

enj · 2019-01-17T14:42:03Z

/refresh

openshift-ci-robot · 2019-01-17T17:16:45Z

New changes are detected. LGTM label has been removed.

openshift-ci-robot · 2019-01-17T17:16:56Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ericavonb, sallyom

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [ericavonb]

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

petr-muller · 2019-01-18T14:04:34Z

/refresh

sallyom · 2019-01-18T16:03:33Z

/retest

enj · 2019-01-21T14:40:19Z

#34

openshift-ci-robot · 2019-01-21T15:30:05Z

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

Test name	Commit	Details	Rerun command
ci/prow/e2e-aws	`da42a7e`	link	`/test e2e-aws`
ci/prow/e2e-aws-operator	`da42a7e`	link	`/test e2e-aws-operator`

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.

Details

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.

openshift-ci-robot requested review from ericavonb and soltysh January 11, 2019 14:36

sallyom force-pushed the cluster-osin-to-authentication branch from ad9c844 to 1ad5727 Compare January 13, 2019 17:19

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

sallyom changed the title ~~WIP replace cluster-osin-operator/cluster-authentication-operator~~ replace cluster-osin-operator/cluster-authentication-operator Jan 13, 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 Jan 13, 2019

sallyom force-pushed the cluster-osin-to-authentication branch 3 times, most recently from 2b9d8d7 to 6e7ab78 Compare January 14, 2019 20:34

sallyom changed the title ~~replace cluster-osin-operator/cluster-authentication-operator~~ rename cluster-osin-operator/cluster-authentication-operator Jan 14, 2019

sallyom force-pushed the cluster-osin-to-authentication branch 2 times, most recently from 2d7b082 to bc86875 Compare January 15, 2019 21:13

enj suggested changes Jan 15, 2019

View reviewed changes

sallyom force-pushed the cluster-osin-to-authentication branch from bc86875 to bc05398 Compare January 15, 2019 23:28

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 Jan 15, 2019

sallyom force-pushed the cluster-osin-to-authentication branch from bc05398 to 76436e7 Compare January 15, 2019 23:39

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

sallyom force-pushed the cluster-osin-to-authentication branch from 76436e7 to e335abb Compare January 16, 2019 00:11

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

sallyom force-pushed the cluster-osin-to-authentication branch 3 times, most recently from 4213a55 to 09cff91 Compare January 16, 2019 19:00

sallyom force-pushed the cluster-osin-to-authentication branch from 09cff91 to 1ac9642 Compare January 16, 2019 19:11

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

openshift-ci-robot assigned ericavonb Jan 16, 2019

ericavonb approved these changes Jan 16, 2019

View reviewed changes

openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 16, 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 Jan 16, 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 Jan 16, 2019

sallyom force-pushed the cluster-osin-to-authentication branch from 1ac9642 to d94b8c0 Compare January 17, 2019 17:16

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jan 17, 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 Jan 17, 2019

sallyom force-pushed the cluster-osin-to-authentication branch from 3b2bede to 70125ad Compare January 18, 2019 13:18

sallyom added 2 commits January 21, 2019 09:40

replace cluster-osin-operator/cluster-authentication-operator

0408f8f

rename imports cluster-osin to cluster-authentication

da42a7e

sallyom force-pushed the cluster-osin-to-authentication branch from 70125ad to da42a7e Compare January 21, 2019 14:40

enj closed this Jan 21, 2019

rename cluster-osin-operator/cluster-authentication-operator #17

rename cluster-osin-operator/cluster-authentication-operator #17

Uh oh!

Conversation

sallyom commented Jan 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ericavonb commented Jan 15, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sallyom Jan 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sallyom Jan 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

enj commented Jan 15, 2019

Uh oh!

ericavonb left a comment

Choose a reason for hiding this comment

Uh oh!

sallyom commented Jan 16, 2019

Uh oh!

sallyom commented Jan 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

enj commented Jan 17, 2019

Uh oh!

openshift-ci-robot commented Jan 17, 2019

Uh oh!

openshift-ci-robot commented Jan 17, 2019

Uh oh!

petr-muller commented Jan 18, 2019

Uh oh!

sallyom commented Jan 18, 2019

Uh oh!

enj commented Jan 21, 2019

Uh oh!

openshift-ci-robot commented Jan 21, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

sallyom commented Jan 11, 2019 •

edited

Loading

sallyom Jan 16, 2019 •

edited

Loading

sallyom Jan 16, 2019 •

edited

Loading

sallyom commented Jan 16, 2019 •

edited

Loading