yugabyte / yugabyte-operator

Kubernetes Operator for YugabyteDB (legacy)
65 stars 29 forks source link

[BUG] Unable to turn off tserver UI service due to inconsistencies in the CRD #33

Open srteam2020 opened 3 years ago

srteam2020 commented 3 years ago

Describe the bug

The code in func (r *ReconcileYBCluster) reconcileUIServices(cluster *yugabytev1alpha1.YBCluster) indicates that we can turn off the tserver UI service by changing the port number to <= 0:

} else if err == nil && cluster.Spec.Tserver.TserverUIPort <= 0 {
    // Delete the service if it existed before & it is not needed going forward.
    logger.Info("deleting tserver ui service")
    if err := r.client.Delete(context.TODO(), found); err != nil {
        logger.Errorf("failed to delete tserver ui service object. err: %+v", err)
        return err
    }
}

However, we find the feature is actually not supported as the min value of Spec.Tserver.TserverUIPort specified in the CRD (yugabyte.com_ybclusters_crd.yaml) is 1. Kubernetes will prevent the users from trying to set the TserverUIPort to <= 0, so the tserver UI service will never be turned off.

A follow-up question: In convention, port 0 is a wildcard port that tells the system to find a suitable port number. So maybe setting cluster.Spec.Tserver.TserverUIPort to -1 would be a better choice for deleting the service.

To reproduce

  1. Create YBCluster with cluster.Spec.Tserver.TserverUIPort set to a 7000, which creates the tserverUI service.
  2. Set Spec.Tserver.TserverUIPort to -1 to delete the tserverUI service. The will not work as -1 is a forbidden value for this field by Kubernetes.

Expected behavior

Users should be able to turn off the tserverUI service by setting Spec.Tserver.TserverUIPort to a negative value as indicated in the code.

Fix

We are willing to issue a fix to solve this inconsistency. We can just remove the min: 1 constraint on Spec.Tserver.TserverUIPort or change the min value to -1 in yugabyte.com_ybclusters_crd.yaml.

srteam2020 commented 3 years ago

We sent a PR to remove the min value for TserverUIPort here: https://github.com/yugabyte/yugabyte-operator/pull/34