Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

reconcile sts/deploy if replica count changed #115

Merged
merged 1 commit into from
Dec 16, 2020
Merged

reconcile sts/deploy if replica count changed #115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 40 additions & 13 deletions controllers/druid/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func deployDruidCluster(sdk client.Client, m *v1alpha1.Druid) error {
if _, err := sdkCreateOrUpdateAsNeeded(sdk,
func() (object, error) { return makeCommonConfigMap(m, ls) },
func() object { return makeConfigMapEmptyObj() },
nil, m, configMapNames); err != nil {
alwaysTrueIsEqualsFn, noopUpdaterFn, m, configMapNames); err != nil {
return err
}

Expand Down Expand Up @@ -112,7 +112,7 @@ func deployDruidCluster(sdk client.Client, m *v1alpha1.Druid) error {
if _, err := sdkCreateOrUpdateAsNeeded(sdk,
func() (object, error) { return nodeConfig, nil },
func() object { return makeConfigMapEmptyObj() },
nil, m, configMapNames); err != nil {
alwaysTrueIsEqualsFn, noopUpdaterFn, m, configMapNames); err != nil {
return err
}

Expand All @@ -122,7 +122,7 @@ func deployDruidCluster(sdk client.Client, m *v1alpha1.Druid) error {
for _, svc := range services {
if _, err := sdkCreateOrUpdateAsNeeded(sdk,
func() (object, error) { return makeService(&svc, &nodeSpec, m, lm, nodeSpecUniqueStr) },
func() object { return makeServiceEmptyObj() },
func() object { return makeServiceEmptyObj() }, alwaysTrueIsEqualsFn,
func(prev, curr object) { (curr.(*v1.Service)).Spec.ClusterIP = (prev.(*v1.Service)).Spec.ClusterIP },
m, serviceNames); err != nil {
return err
Expand All @@ -140,7 +140,7 @@ func deployDruidCluster(sdk client.Client, m *v1alpha1.Druid) error {
return makeDeployment(&nodeSpec, m, lm, nodeSpecUniqueStr, fmt.Sprintf("%s-%s", commonConfigSHA, nodeConfigSHA), firstServiceName)
},
func() object { return makeDeploymentEmptyObj() },
nil, m, deploymentNames); err != nil {
deploymentIsEquals, noopUpdaterFn, m, deploymentNames); err != nil {
return err
} else if m.Spec.RollingDeploy {

Expand All @@ -161,7 +161,7 @@ func deployDruidCluster(sdk client.Client, m *v1alpha1.Druid) error {
return makeStatefulSet(&nodeSpec, m, lm, nodeSpecUniqueStr, fmt.Sprintf("%s-%s", commonConfigSHA, nodeConfigSHA), firstServiceName)
},
func() object { return makeStatefulSetEmptyObj() },
nil, m, statefulSetNames); err != nil {
statefulSetIsEquals, noopUpdaterFn, m, statefulSetNames); err != nil {
return err
} else if m.Spec.RollingDeploy {

Expand Down Expand Up @@ -192,7 +192,7 @@ func deployDruidCluster(sdk client.Client, m *v1alpha1.Druid) error {
return makeIngress(&nodeSpec, m, ls, nodeSpecUniqueStr)
},
func() object { return makeIngressEmptyObj() },
nil, m, ingressNames); err != nil {
alwaysTrueIsEqualsFn, noopUpdaterFn, m, ingressNames); err != nil {
return err
}
}
Expand All @@ -202,7 +202,7 @@ func deployDruidCluster(sdk client.Client, m *v1alpha1.Druid) error {
if _, err := sdkCreateOrUpdateAsNeeded(sdk,
func() (object, error) { return makePodDisruptionBudget(&nodeSpec, m, lm, nodeSpecUniqueStr) },
func() object { return makePodDisruptionBudgetEmptyObj() },
nil, m, podDisruptionBudgetNames); err != nil {
alwaysTrueIsEqualsFn, noopUpdaterFn, m, podDisruptionBudgetNames); err != nil {
return err
}
}
Expand All @@ -214,7 +214,7 @@ func deployDruidCluster(sdk client.Client, m *v1alpha1.Druid) error {
return makeHorizontalPodAutoscaler(&nodeSpec, m, ls, nodeSpecUniqueStr)
},
func() object { return makeHorizontalPodAutoscalerEmptyObj() },
nil, m, hpaNames); err != nil {
alwaysTrueIsEqualsFn, noopUpdaterFn, m, hpaNames); err != nil {
return err
}
}
Expand Down Expand Up @@ -431,7 +431,22 @@ type object interface {
runtime.Object
}

func sdkCreateOrUpdateAsNeeded(sdk client.Client, objFn func() (object, error), emptyObjFn func() object, updaterFn func(prev, curr object), drd *v1alpha1.Druid, names map[string]bool) (string, error) {
func alwaysTrueIsEqualsFn(prev, curr object) bool {
return true
}

func noopUpdaterFn(prev, curr object) {
// do nothing
}

func sdkCreateOrUpdateAsNeeded(
sdk client.Client,
objFn func() (object, error),
emptyObjFn func() object,
isEqualFn func(prev, curr object) bool,
updaterFn func(prev, curr object),
drd *v1alpha1.Druid,
names map[string]bool) (string, error) {
if obj, err := objFn(); err != nil {
return "", err
} else {
Expand Down Expand Up @@ -463,12 +478,10 @@ func sdkCreateOrUpdateAsNeeded(sdk client.Client, objFn func() (object, error),
}
} else {
// resource already exists, updated it if needed
if obj.GetAnnotations()[druidOpResourceHash] != prevObj.GetAnnotations()[druidOpResourceHash] {
if obj.GetAnnotations()[druidOpResourceHash] != prevObj.GetAnnotations()[druidOpResourceHash] || !isEqualFn(prevObj, obj) {

obj.SetResourceVersion(prevObj.GetResourceVersion())
if updaterFn != nil {
updaterFn(prevObj, obj)
}
updaterFn(prevObj, obj)

if err := sdk.Update(context.TODO(), obj); err != nil {
e := fmt.Errorf("Failed to update [%s:%s] due to [%s].", obj.GetObjectKind().GroupVersionKind().Kind, obj.GetName(), err.Error())
Expand Down Expand Up @@ -831,6 +844,13 @@ func makeStatefulSet(nodeSpec *v1alpha1.DruidNodeSpec, m *v1alpha1.Druid, ls map
}, nil
}

// currently only checks for replica count mismatch, can be extended further
func statefulSetIsEquals(obj1, obj2 object) bool {
o1 := obj1.(*appsv1.StatefulSet)
o2 := obj2.(*appsv1.StatefulSet)
return *o1.Spec.Replicas == *o2.Spec.Replicas
}

Comment on lines +848 to +853
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@himanshug in case we add spec here, wonder if this will conflict on scaling sts #97

Regardless are you planning to keep only replicas for now or extend for spec too

Copy link
Member Author

@himanshug himanshug Dec 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for now, I am leaving it at just replica count mismatch ... but we can add more stuff here based on real world needs.

from feature perspective .. #97 and this , are different things ... however both might end up reusing some code for checking equality if that makes sense.

// makeDeployment shall create deployment object.
func makeDeployment(nodeSpec *v1alpha1.DruidNodeSpec, m *v1alpha1.Druid, ls map[string]string, nodeSpecUniqueStr, configMapSHA, serviceName string) (*appsv1.Deployment, error) {
return &appsv1.Deployment{
Expand All @@ -847,6 +867,13 @@ func makeDeployment(nodeSpec *v1alpha1.DruidNodeSpec, m *v1alpha1.Druid, ls map[
}, nil
}

// currently only checks for replica count mismatch, can be extended further
func deploymentIsEquals(obj1, obj2 object) bool {
o1 := obj1.(*appsv1.Deployment)
o2 := obj2.(*appsv1.Deployment)
return *o1.Spec.Replicas == *o2.Spec.Replicas
}

// makeStatefulSetSpec shall create statefulset spec for statefulsets.
func makeStatefulSetSpec(nodeSpec *v1alpha1.DruidNodeSpec, m *v1alpha1.Druid, ls map[string]string, nodeSpecificUniqueString, configMapSHA, serviceName string) appsv1.StatefulSetSpec {

Expand Down
2 changes: 0 additions & 2 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ go 1.13
require (
github.com/ghodss/yaml v1.0.0
github.com/go-logr/logr v0.1.0
github.com/onsi/ginkgo v1.11.0
github.com/onsi/gomega v1.8.1
github.com/stretchr/testify v1.4.0
k8s.io/api v0.18.2
k8s.io/apimachinery v0.18.2
Expand Down