Refactoring elasticsearch operator to improve code readability #80

ewolinetz · 2019-02-14T21:02:38Z

This may look noisy until #81 is merged in

richm · 2019-02-17T19:49:22Z

/retest

richm · 2019-02-18T03:38:19Z

/retest

ewolinetz · 2019-03-14T15:46:37Z

/test e2e-aws

ewolinetz · 2019-03-14T21:03:22Z

/retest

ewolinetz · 2019-03-15T13:41:40Z

/test e2e-operator

pkg/k8shandler/common.go

jcantrill

I think we can still tighten things up but I'm good with a first pass if we want to merge

jcantrill · 2019-03-15T17:58:24Z

pkg/k8shandler/common.go

+
+	return []v1.EnvVar{
+		v1.EnvVar{
+			Name:  "DC_NAME",


Again in future, maybe we should consider changing this to DEPLOYMENT_NAME since we are not using DC

jcantrill · 2019-03-15T17:58:56Z

pkg/k8shandler/common.go

+			Value: fmt.Sprintf("cluster=%s", clusterName),
+		},
+		v1.EnvVar{
+			Name:  "IS_MASTER",


pkg/k8shandler/common.go

jcantrill · 2019-03-15T18:02:12Z

pkg/k8shandler/defaults.go

+
+	defaultMasterCPULimit     = "100m"
+	defaultMasterCPURequest   = "100m"
+	defaultCPULimit           = "4000m"


we removed the cpu limit I think in 3.9

ewolinetz · 2019-03-18T14:47:52Z

@jcantrill looks like we had cpu limit and cpu request in 3.9...
https://github.com/openshift/openshift-ansible/blob/release-3.9/roles/openshift_logging_elasticsearch/templates/5.x/es.j2#L62

ewolinetz · 2019-03-18T16:43:57Z

/hold
going to add a fix for https://bugzilla.redhat.com/show_bug.cgi?id=1689762 as part of this

ewolinetz · 2019-03-18T18:32:19Z

pkg/apis/elasticsearch/v1alpha1/types.go


 type ElasticsearchStorageSpec struct {
-	StorageClassName string             `json:"storageClassName,omitempty"`
+	StorageClassName *string            `json:"storageClassName,omitempty"`


this is part of addressing https://bugzilla.redhat.com/show_bug.cgi?id=1689762
this may break CLO when we specify a storageClassName -- will create a follow up fix

will need to do a dep ensure -update for this

ewolinetz · 2019-03-18T18:47:05Z

/hold cancel

jcantrill

There is a lot here and I think more to refactor via smaller chunks. Assuming there are no regressions I would like to get this in and then work from there.

ewolinetz · 2019-03-18T20:20:29Z

/test e2e-aws

ewolinetz · 2019-03-19T22:06:20Z

/retest

richm · 2019-03-19T23:46:26Z

/retest

richm · 2019-03-20T00:48:25Z

/retest

richm · 2019-03-20T02:01:29Z

/retest

jcantrill

/lgtm

openshift-ci-robot · 2019-03-20T14:25:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ewolinetz, jcantrill

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~~ [ewolinetz,jcantrill]

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

ewolinetz · 2019-03-20T14:26:24Z

/hold

openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 14, 2019

openshift-ci-robot requested review from jcantrill and richm February 14, 2019 21:05

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

ewolinetz force-pushed the eo_refactoring branch 5 times, most recently from f9f322c to 9b703eb Compare February 14, 2019 21:30

openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 14, 2019

ewolinetz force-pushed the eo_refactoring branch from 9b703eb to f04a3f6 Compare February 14, 2019 22:19

ewolinetz force-pushed the eo_refactoring branch from ee87e73 to 3f413b9 Compare February 18, 2019 15:25

openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 18, 2019

ewolinetz force-pushed the eo_refactoring branch 2 times, most recently from 4f115fc to 002fca9 Compare February 18, 2019 18:25

openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 18, 2019

ewolinetz force-pushed the eo_refactoring branch 4 times, most recently from 2881745 to d93bb07 Compare February 19, 2019 16:52

ewolinetz force-pushed the eo_refactoring branch from 9ef0e3a to 189d754 Compare March 14, 2019 14:07

ewolinetz force-pushed the eo_refactoring branch from 189d754 to 20f5e23 Compare March 14, 2019 19:35

jcantrill reviewed Mar 15, 2019

View reviewed changes

pkg/k8shandler/common.go Show resolved Hide resolved

jcantrill approved these changes Mar 15, 2019

View reviewed changes

ewolinetz force-pushed the eo_refactoring branch from 20f5e23 to e2c9230 Compare March 18, 2019 18:24

ewolinetz commented Mar 18, 2019

View reviewed changes

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

jcantrill approved these changes Mar 18, 2019

View reviewed changes

Refactoring elasticsearch operator to improve code readability

681bf5b

ewolinetz force-pushed the eo_refactoring branch from e2c9230 to 681bf5b Compare March 19, 2019 21:20

jcantrill approved these changes Mar 20, 2019

View reviewed changes

openshift-ci-robot assigned jcantrill Mar 20, 2019

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

openshift-merge-robot merged commit b8b3e00 into openshift:master Mar 20, 2019

ewolinetz mentioned this pull request Mar 20, 2019

Updating to work with change in StorageClassName type from EO openshift/cluster-logging-operator#125

Merged

ewolinetz deleted the eo_refactoring branch December 1, 2020 17:09

Refactoring elasticsearch operator to improve code readability #80

Refactoring elasticsearch operator to improve code readability #80

Uh oh!

Conversation

ewolinetz commented Feb 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

richm commented Feb 17, 2019

Uh oh!

richm commented Feb 18, 2019

Uh oh!

ewolinetz commented Mar 14, 2019

Uh oh!

ewolinetz commented Mar 14, 2019

Uh oh!

ewolinetz commented Mar 15, 2019

Uh oh!

Uh oh!

jcantrill left a comment

Choose a reason for hiding this comment

Uh oh!

jcantrill Mar 15, 2019

Choose a reason for hiding this comment

Uh oh!

jcantrill Mar 15, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jcantrill Mar 15, 2019

Choose a reason for hiding this comment

Uh oh!

ewolinetz commented Mar 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ewolinetz commented Mar 18, 2019

Uh oh!

ewolinetz Mar 18, 2019

Choose a reason for hiding this comment

Uh oh!

ewolinetz Mar 18, 2019

Choose a reason for hiding this comment

Uh oh!

ewolinetz commented Mar 18, 2019

Uh oh!

jcantrill left a comment

Choose a reason for hiding this comment

Uh oh!

ewolinetz commented Mar 18, 2019

Uh oh!

ewolinetz commented Mar 19, 2019

Uh oh!

richm commented Mar 19, 2019

Uh oh!

richm commented Mar 20, 2019

Uh oh!

richm commented Mar 20, 2019

Uh oh!

jcantrill left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Mar 20, 2019

Uh oh!

ewolinetz commented Mar 20, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ewolinetz commented Feb 14, 2019 •

edited

Loading

ewolinetz commented Mar 18, 2019 •

edited

Loading