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.

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

add hash to end of resource names to avoid name clash #605

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ func setupRayClusterController(mgr ctrl.Manager, cfg *config.CodeFlareOperatorCo
<-certsReady
setupLog.Info("Certs ready")

err := controllers.SetupRayClusterWebhookWithManager(mgr, cfg.KubeRay)
err := controllers.SetupRayClusterWebhookWithManager(mgr, cfg.KubeRay, OperatorVersion)
if err != nil {
return err
}
Expand Down
60 changes: 52 additions & 8 deletions pkg/controllers/raycluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,12 @@ var (
// For more details, check Reconcile and its Result here:
// - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/reconcile

func shouldUseOldName(cluster *rayv1.RayCluster) bool {
// hashed name code was added in the same commit as the version annotation
_, ok := cluster.GetAnnotations()[versionAnnotation]
return !ok
}

func (r *RayClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
logger := ctrl.LoggerFrom(ctx)

Expand Down Expand Up @@ -304,7 +310,10 @@ func isMTLSEnabled(cfg *config.KubeRayConfiguration) bool {
}

func crbNameFromCluster(cluster *rayv1.RayCluster) string {
return cluster.Name + "-" + cluster.Namespace + "-auth" // NOTE: potential naming conflicts ie {name: foo, ns: bar-baz} and {name: foo-bar, ns: baz}
if shouldUseOldName(cluster) {
return cluster.Name + "-" + cluster.Namespace + "-auth"
}
return RCCUniqueName(cluster.Name + "-" + cluster.Namespace + "-auth")
}

func desiredOAuthClusterRoleBinding(cluster *rayv1.RayCluster) *rbacv1ac.ClusterRoleBindingApplyConfiguration {
Expand All @@ -326,7 +335,10 @@ func desiredOAuthClusterRoleBinding(cluster *rayv1.RayCluster) *rbacv1ac.Cluster
}

func oauthServiceAccountNameFromCluster(cluster *rayv1.RayCluster) string {
return cluster.Name + "-oauth-proxy"
if shouldUseOldName(cluster) {
return cluster.Name + "-oauth-proxy"
}
return RCCUniqueName(cluster.Name + "-oauth-proxy")
}

func desiredServiceAccount(cluster *rayv1.RayCluster) *corev1ac.ServiceAccountApplyConfiguration {
Expand Down Expand Up @@ -363,11 +375,17 @@ func desiredClusterRoute(cluster *rayv1.RayCluster) *routev1ac.RouteApplyConfigu
}

func oauthServiceNameFromCluster(cluster *rayv1.RayCluster) string {
return cluster.Name + "-oauth"
if shouldUseOldName(cluster) {
return cluster.Name + "-oauth"
}
return RCCUniqueName(cluster.Name + "-oauth")
}

func oauthServiceTLSSecretName(cluster *rayv1.RayCluster) string {
return cluster.Name + "-proxy-tls-secret"
if shouldUseOldName(cluster) {
return cluster.Name + "-proxy-tls-secret"
}
return RCCUniqueName(cluster.Name + "-proxy-tls-secret")
}

func desiredOAuthService(cluster *rayv1.RayCluster) *corev1ac.ServiceApplyConfiguration {
Expand All @@ -389,7 +407,10 @@ func desiredOAuthService(cluster *rayv1.RayCluster) *corev1ac.ServiceApplyConfig
}

func oauthSecretNameFromCluster(cluster *rayv1.RayCluster) string {
return cluster.Name + "-oauth-config"
if shouldUseOldName(cluster) {
return cluster.Name + "-oauth-config"
}
return RCCUniqueName(cluster.Name + "-oauth-config")
}

// desiredOAuthSecret defines the desired OAuth secret object
Expand All @@ -406,7 +427,10 @@ func desiredOAuthSecret(cluster *rayv1.RayCluster, cookieSalt string) *corev1ac.
}

func caSecretNameFromCluster(cluster *rayv1.RayCluster) string {
return "ca-secret-" + cluster.Name
if shouldUseOldName(cluster) {
return "ca-secret-" + cluster.Name
}
return RCCUniqueName(cluster.Name + "-ca-secret")
}

func desiredCASecret(cluster *rayv1.RayCluster, key, cert []byte) *corev1ac.SecretApplyConfiguration {
Expand Down Expand Up @@ -462,8 +486,17 @@ func generateCACertificate() ([]byte, []byte, error) {
return privateKeyPem, certPem, nil
}

func workerNWPNameFromCluster(cluster *rayv1.RayCluster) string {
if shouldUseOldName(cluster) {
return cluster.Name + "-workers"
}
return RCCUniqueName(cluster.Name + "-workers")
}

func desiredWorkersNetworkPolicy(cluster *rayv1.RayCluster) *networkingv1ac.NetworkPolicyApplyConfiguration {
return networkingv1ac.NetworkPolicy(cluster.Name+"-workers", cluster.Namespace).
return networkingv1ac.NetworkPolicy(
workerNWPNameFromCluster(cluster), cluster.Namespace,
).
WithLabels(map[string]string{RayClusterNameLabel: cluster.Name}).
WithSpec(networkingv1ac.NetworkPolicySpec().
WithPodSelector(metav1ac.LabelSelector().WithMatchLabels(map[string]string{"ray.io/cluster": cluster.Name, "ray.io/node-type": "worker"})).
Expand All @@ -477,14 +510,21 @@ func desiredWorkersNetworkPolicy(cluster *rayv1.RayCluster) *networkingv1ac.Netw
WithOwnerReferences(ownerRefForRayCluster(cluster))
}

func headNWPNameFromCluster(cluster *rayv1.RayCluster) string {
if shouldUseOldName(cluster) {
return cluster.Name + "-head"
}
return RCCUniqueName(cluster.Name + "-head")
}

func desiredHeadNetworkPolicy(cluster *rayv1.RayCluster, cfg *config.KubeRayConfiguration, kubeRayNamespaces []string) *networkingv1ac.NetworkPolicyApplyConfiguration {
allSecuredPorts := []*networkingv1ac.NetworkPolicyPortApplyConfiguration{
networkingv1ac.NetworkPolicyPort().WithProtocol(corev1.ProtocolTCP).WithPort(intstr.FromInt(8443)),
}
if ptr.Deref(cfg.MTLSEnabled, true) {
allSecuredPorts = append(allSecuredPorts, networkingv1ac.NetworkPolicyPort().WithProtocol(corev1.ProtocolTCP).WithPort(intstr.FromInt(10001)))
}
return networkingv1ac.NetworkPolicy(cluster.Name+"-head", cluster.Namespace).
return networkingv1ac.NetworkPolicy(headNWPNameFromCluster(cluster), cluster.Namespace).
WithLabels(map[string]string{RayClusterNameLabel: cluster.Name}).
WithSpec(networkingv1ac.NetworkPolicySpec().
WithPodSelector(metav1ac.LabelSelector().WithMatchLabels(map[string]string{"ray.io/cluster": cluster.Name, "ray.io/node-type": "head"})).
Expand Down Expand Up @@ -619,3 +659,7 @@ func (r *RayClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {

return controller.Complete(r)
}

func RCCUniqueName(s string) string {
return s + "-" + seededHash(controllerName, s)
}
34 changes: 23 additions & 11 deletions pkg/controllers/raycluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,16 @@ const (
oauthProxyContainerName = "oauth-proxy"
oauthProxyVolumeName = "proxy-tls-secret"
initContainerName = "create-cert"
versionAnnotation = "ray.openshift.ai/version"
)

// log is for logging in this package.
var rayclusterlog = logf.Log.WithName("raycluster-resource")

func SetupRayClusterWebhookWithManager(mgr ctrl.Manager, cfg *config.KubeRayConfiguration) error {
func SetupRayClusterWebhookWithManager(mgr ctrl.Manager, cfg *config.KubeRayConfiguration, operatorVersion string) error {
rayClusterWebhookInstance := &rayClusterWebhook{
Config: cfg,
Config: cfg,
OperatorVersion: operatorVersion,
}
return ctrl.NewWebhookManagedBy(mgr).
For(&rayv1.RayCluster{}).
Expand All @@ -58,23 +60,33 @@ func SetupRayClusterWebhookWithManager(mgr ctrl.Manager, cfg *config.KubeRayConf
// +kubebuilder:webhook:path=/validate-ray-io-v1-raycluster,mutating=false,failurePolicy=fail,sideEffects=None,groups=ray.io,resources=rayclusters,verbs=create;update,versions=v1,name=vraycluster.ray.openshift.ai,admissionReviewVersions=v1

type rayClusterWebhook struct {
Config *config.KubeRayConfiguration
Config *config.KubeRayConfiguration
OperatorVersion string
}

var _ webhook.CustomDefaulter = &rayClusterWebhook{}
var _ webhook.CustomValidator = &rayClusterWebhook{}

// Default implements webhook.Defaulter so a webhook will be registered for the type
func (w *rayClusterWebhook) Default(ctx context.Context, obj runtime.Object) error {
logger := ctrl.LoggerFrom(ctx)
rayCluster := obj.(*rayv1.RayCluster)

// add annotation to use new names
annotations := rayCluster.GetAnnotations()
if annotations == nil {
annotations = make(map[string]string)
}
annotations[versionAnnotation] = w.OperatorVersion
rayCluster.SetAnnotations(annotations)
logger.Info("Ray Cluster annotations", "annotations", rayCluster.GetAnnotations())
if ptr.Deref(w.Config.RayDashboardOAuthEnabled, true) {
rayclusterlog.V(2).Info("Adding OAuth sidecar container")
rayCluster.Spec.HeadGroupSpec.Template.Spec.Containers = upsert(rayCluster.Spec.HeadGroupSpec.Template.Spec.Containers, oauthProxyContainer(rayCluster), withContainerName(oauthProxyContainerName))

rayCluster.Spec.HeadGroupSpec.Template.Spec.Volumes = upsert(rayCluster.Spec.HeadGroupSpec.Template.Spec.Volumes, oauthProxyTLSSecretVolume(rayCluster), withVolumeName(oauthProxyVolumeName))

rayCluster.Spec.HeadGroupSpec.Template.Spec.ServiceAccountName = rayCluster.Name + "-oauth-proxy"
rayCluster.Spec.HeadGroupSpec.Template.Spec.ServiceAccountName = oauthServiceAccountNameFromCluster(rayCluster)
}

if ptr.Deref(w.Config.MTLSEnabled, true) {
Expand Down Expand Up @@ -218,7 +230,7 @@ func validateIngress(rayCluster *rayv1.RayCluster) field.ErrorList {
func validateHeadGroupServiceAccountName(rayCluster *rayv1.RayCluster) field.ErrorList {
var allErrors field.ErrorList

if rayCluster.Spec.HeadGroupSpec.Template.Spec.ServiceAccountName != rayCluster.Name+"-oauth-proxy" {
if rayCluster.Spec.HeadGroupSpec.Template.Spec.ServiceAccountName != oauthServiceAccountNameFromCluster(rayCluster) {
allErrors = append(allErrors, field.Invalid(
field.NewPath("spec", "headGroupSpec", "template", "spec", "serviceAccountName"),
rayCluster.Spec.HeadGroupSpec.Template.Spec.ServiceAccountName,
Expand All @@ -241,7 +253,7 @@ func oauthProxyContainer(rayCluster *rayv1.RayCluster) corev1.Container {
ValueFrom: &corev1.EnvVarSource{
SecretKeyRef: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{
Name: rayCluster.Name + "-oauth-config",
Name: oauthSecretNameFromCluster(rayCluster),
},
Key: "cookie_secret",
},
Expand All @@ -251,7 +263,7 @@ func oauthProxyContainer(rayCluster *rayv1.RayCluster) corev1.Container {
Args: []string{
"--https-address=:8443",
"--provider=openshift",
"--openshift-service-account=" + rayCluster.Name + "-oauth-proxy",
"--openshift-service-account=" + oauthServiceAccountNameFromCluster(rayCluster),
"--upstream=http://localhost:8265",
"--tls-cert=/etc/tls/private/tls.crt",
"--tls-key=/etc/tls/private/tls.key",
Expand All @@ -273,7 +285,7 @@ func oauthProxyTLSSecretVolume(rayCluster *rayv1.RayCluster) corev1.Volume {
Name: oauthProxyVolumeName,
VolumeSource: corev1.VolumeSource{
Secret: &corev1.SecretVolumeSource{
SecretName: rayCluster.Name + "-proxy-tls-secret",
SecretName: oauthServiceTLSSecretName(rayCluster),
},
},
}
Expand Down Expand Up @@ -329,7 +341,7 @@ func caVolumes(rayCluster *rayv1.RayCluster) []corev1.Volume {
Name: "ca-vol",
VolumeSource: corev1.VolumeSource{
Secret: &corev1.SecretVolumeSource{
SecretName: `ca-secret-` + rayCluster.Name,
SecretName: caSecretNameFromCluster(rayCluster),
},
},
},
Expand All @@ -343,9 +355,9 @@ func caVolumes(rayCluster *rayv1.RayCluster) []corev1.Volume {
}

func rayHeadInitContainer(rayCluster *rayv1.RayCluster, config *config.KubeRayConfiguration) corev1.Container {
rayClientRoute := "rayclient-" + rayCluster.Name + "-" + rayCluster.Namespace + "." + config.IngressDomain
rayClientRoute := rayClientNameFromCluster(rayCluster) + "-" + rayCluster.Namespace + "." + config.IngressDomain
// Service name for basic interactive
svcDomain := rayCluster.Name + "-head-svc." + rayCluster.Namespace + ".svc"
svcDomain := serviceNameFromCluster(rayCluster) + "." + rayCluster.Namespace + ".svc"

initContainerHead := corev1.Container{
Name: "create-cert",
Expand Down
Loading