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

issue-464:Added support for JvmOption customization #462

Merged
merged 9 commits into from
Oct 20, 2020
Merged

issue-464:Added support for JvmOption customization #462

Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
eda074a
added support for jvmOption customization
Oct 12, 2020
f7a1df1
removed sleep
Oct 12, 2020
3d1e53c
added timeout and made restart pod function modular as well
Oct 12, 2020
7ac6948
changed restart logic for controller pods
Oct 12, 2020
92df785
added reconcileconfigmap and validation
Oct 16, 2020
a469512
ut for compareConfigMap added
Oct 16, 2020
d29cd94
adding e2e for cm changes added
Oct 19, 2020
4a2632c
more validation added
Oct 19, 2020
991a6fe
fixed typo
Oct 20, 2020
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
164 changes: 158 additions & 6 deletions pkg/controller/pravegacluster/pravegacluster_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"context"
"fmt"
"strconv"
"strings"
"time"

pravegav1beta1 "github.com/pravega/pravega-operator/pkg/apis/pravega/v1beta1"
Expand Down Expand Up @@ -316,11 +317,32 @@ func (r *ReconcilePravegaCluster) deployController(p *pravegav1beta1.PravegaClus
return err
}

var eq bool = true
currentConfigMap := &corev1.ConfigMap{}
configMap := pravega.MakeControllerConfigMap(p)
controllerutil.SetControllerReference(p, configMap, r.scheme)
err = r.client.Create(context.TODO(), configMap)
if err != nil && !errors.IsAlreadyExists(err) {
return err
err = r.client.Get(context.TODO(), types.NamespacedName{Name: configMap.Name, Namespace: p.Namespace}, currentConfigMap)
Copy link
Contributor

Choose a reason for hiding this comment

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

This does not seem like a good place and a good way to check for updates to config map.
Please follow the logic used in zookeeper -operator:
https://github.com/pravega/zookeeper-operator/blob/master/pkg/controller/zookeepercluster/zookeepercluster_controller.go#L150

In p-operator we're checking very specific things in the reconcile loop, instead of checking for updates to each component and that is complicating the flow of code way too much...
Lets refactor and change it to the way it is done in ZK operator...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

if err != nil {
if errors.IsNotFound(err) {
err = r.client.Create(context.TODO(), configMap)
if err != nil && !errors.IsAlreadyExists(err) {
return err
}
}
} else {
currentConfigMap := &corev1.ConfigMap{}
err = r.client.Get(context.TODO(), types.NamespacedName{Namespace: configMap.Namespace, Name: configMap.Name}, currentConfigMap)
currentjavaOptsList := currentConfigMap.Data["JAVA_OPTS"]
for _, jvmopts := range p.Spec.Pravega.ControllerJvmOptions {
eq = strings.Contains(currentjavaOptsList, jvmopts)
if !eq {
err := r.client.Update(context.TODO(), configMap)
if err != nil {
return err
}
break
}
}
}

deployment := pravega.MakeControllerDeployment(p)
Expand All @@ -329,6 +351,29 @@ func (r *ReconcilePravegaCluster) deployController(p *pravegav1beta1.PravegaClus
if err != nil && !errors.IsAlreadyExists(err) {
return err
}
if !eq {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment adding too many ifs... complicates the flow of code.. I think its time to refactor the reconcile loop...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

currentDeployment := &appsv1.Deployment{}
err := r.client.Get(context.TODO(), types.NamespacedName{Name: deployment.Name, Namespace: deployment.Namespace}, currentDeployment)
selector, err := metav1.LabelSelectorAsSelector(&metav1.LabelSelector{
MatchLabels: deployment.Spec.Template.Labels,
})
if err != nil {
return fmt.Errorf("failed to convert label selector: %v", err)
}
podList := &corev1.PodList{}
podlistOps := &client.ListOptions{
Namespace: deployment.Namespace,
LabelSelector: selector,
}
err = r.client.List(context.TODO(), podList, podlistOps)
if err != nil {
return err
}
err = r.restartDeploymentPod(podList, p)
if err != nil {
return err
}
}

service := pravega.MakeControllerService(p)
controllerutil.SetControllerReference(p, service, r.scheme)
Expand Down Expand Up @@ -367,11 +412,32 @@ func (r *ReconcilePravegaCluster) deploySegmentStore(p *pravegav1beta1.PravegaCl
return err
}

var eq bool = true
currentConfigMap := &corev1.ConfigMap{}
configMap := pravega.MakeSegmentstoreConfigMap(p)
controllerutil.SetControllerReference(p, configMap, r.scheme)
err = r.client.Create(context.TODO(), configMap)
if err != nil && !errors.IsAlreadyExists(err) {
return err
err = r.client.Get(context.TODO(), types.NamespacedName{Name: configMap.Name, Namespace: p.Namespace}, currentConfigMap)
if err != nil {
if errors.IsNotFound(err) {
err = r.client.Create(context.TODO(), configMap)
if err != nil && !errors.IsAlreadyExists(err) {
return err
}
}
} else {
currentConfigMap := &corev1.ConfigMap{}
err = r.client.Get(context.TODO(), types.NamespacedName{Namespace: configMap.Namespace, Name: configMap.Name}, currentConfigMap)
currentjavaOptsList := currentConfigMap.Data["JAVA_OPTS"]
for _, jvmopts := range p.Spec.Pravega.SegmentStoreJVMOptions {
eq = strings.Contains(currentjavaOptsList, jvmopts)
if !eq {
err := r.client.Update(context.TODO(), configMap)
if err != nil {
return err
}
break
}
}
}

statefulSet := pravega.MakeSegmentStoreStatefulSet(p)
Expand Down Expand Up @@ -404,6 +470,29 @@ func (r *ReconcilePravegaCluster) deploySegmentStore(p *pravegav1beta1.PravegaCl
}
}
}
if !eq {
currentSts := &appsv1.StatefulSet{}
err := r.client.Get(context.TODO(), types.NamespacedName{Name: statefulSet.Name, Namespace: statefulSet.Namespace}, currentSts)
selector, err := metav1.LabelSelectorAsSelector(&metav1.LabelSelector{
MatchLabels: statefulSet.Spec.Template.Labels,
})
if err != nil {
return fmt.Errorf("failed to convert label selector: %v", err)
}
podList := &corev1.PodList{}
podlistOps := &client.ListOptions{
Namespace: statefulSet.Namespace,
LabelSelector: selector,
}
err = r.client.List(context.TODO(), podList, podlistOps)
if err != nil {
return err
}
err = r.restartStsPod(podList)
if err != nil {
return err
}
}
return nil
}

Expand All @@ -416,6 +505,69 @@ func hasOldVersionOwnerReference(ownerreference []metav1.OwnerReference) bool {
return false
}

func (r *ReconcilePravegaCluster) restartStsPod(podList *corev1.PodList) error {
for _, podItem := range podList.Items {
err := r.client.Delete(context.TODO(), &podItem)
if err != nil {
return err
} else {
start := time.Now()
pod := &corev1.Pod{}
err = r.client.Get(context.TODO(), types.NamespacedName{Name: podItem.ObjectMeta.Name, Namespace: podItem.ObjectMeta.Namespace}, pod)
for util.IsPodReady(pod) {
if time.Since(start) > 10*time.Minute {
return fmt.Errorf("failed to delete Segmentstore pod (%s) for 10 mins ", podItem.ObjectMeta.Name)
}
err = r.client.Get(context.TODO(), types.NamespacedName{Name: podItem.ObjectMeta.Name, Namespace: podItem.ObjectMeta.Namespace}, pod)
}
start = time.Now()
err = r.client.Get(context.TODO(), types.NamespacedName{Name: podItem.ObjectMeta.Name, Namespace: podItem.ObjectMeta.Namespace}, pod)
for !util.IsPodReady(pod) {
if time.Since(start) > 10*time.Minute {
return fmt.Errorf("failed to get Segmentstore pod (%s) as ready for 10 mins ", podItem.ObjectMeta.Name)
}
err = r.client.Get(context.TODO(), types.NamespacedName{Name: podItem.ObjectMeta.Name, Namespace: podItem.ObjectMeta.Namespace}, pod)
}
}
}
return nil
}
func (r *ReconcilePravegaCluster) restartDeploymentPod(podList *corev1.PodList, p *pravegav1beta1.PravegaCluster) error {
for _, podItem := range podList.Items {
err := r.client.Delete(context.TODO(), &podItem)
if err != nil {
return err
} else {
start := time.Now()
pod := &corev1.Pod{}
err = r.client.Get(context.TODO(), types.NamespacedName{Name: podItem.ObjectMeta.Name, Namespace: podItem.ObjectMeta.Namespace}, pod)
for util.IsPodReady(pod) {
if time.Since(start) > 10*time.Minute {
return fmt.Errorf("failed to delete controller pod (%s) for 10 mins ", podItem.ObjectMeta.Name)
}
err = r.client.Get(context.TODO(), types.NamespacedName{Name: podItem.ObjectMeta.Name, Namespace: podItem.ObjectMeta.Namespace}, pod)
}
deploy := &appsv1.Deployment{}
name := p.DeploymentNameForController()
err = r.client.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: p.Namespace}, deploy)
if err != nil {
return fmt.Errorf("failed to get deployment (%s): %v", deploy.Name, err)
}
start = time.Now()
for deploy.Status.ReadyReplicas != deploy.Status.Replicas {
if time.Since(start) > 10*time.Minute {
return fmt.Errorf("failed to make controller pod ready for 10 mins ")
}
err = r.client.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: p.Namespace}, deploy)
if err != nil {
return fmt.Errorf("failed to get deployment (%s): %v", deploy.Name, err)
}
}
}
}
return nil
}

func (r *ReconcilePravegaCluster) syncClusterSize(p *pravegav1beta1.PravegaCluster) (err error) {
/*We skip calling syncSegmentStoreSize() during upgrade/rollback from version 07*/
if !r.IsClusterUpgradingTo07(p) && !r.IsClusterRollbackingFrom07(p) {
Expand Down