Create sandbox config maps in sandbox reconciler #790
Conversation
pkg/builder/builder.go
Outdated
| "verticaDBName": vdb.Spec.DBName, | ||
| "sandboxName": sandbox, |
There was a problem hiding this comment.
We should sync with Roy's PR (#785). He added const values for these key names.
| if _, ok := seenScs[v.subclusterName]; ok { | ||
| continue | ||
| } | ||
| if v.sandbox != vapi.MainCluster { |
There was a problem hiding this comment.
I expected there would be a change to podfacts.go. It looks like we set v.sandbox initially in collectPodByStsIndex and we get it from the requested sandbox we are requesting. That doesn't seem right. I feel like we should pull it from a label in the statefulset. The comment seems to indicate that, but I'm not sure if that's exactly what's going on.
This is the code I am talking about:
// we get the sandbox name from the sts labels if the subcluster
// belongs to a sandbox. If the node is up, we will later retrieve
// the sandbox state from the catalog
pf.sandbox = p.SandboxName
p.SandboxName is just an input parameter. I wonder if it should be sts.Labels[vmeta.SandboxNameLabel.
There was a problem hiding this comment.
I will change it to sts.Labels[vmeta.SandboxNameLabel] since it probably causes an error when the node is down. That time we cannot update it to the correct sandbox name from checkNodeDetails. Then it could make the node restart fail.
| } | ||
|
|
||
| // reconcile sandbox config maps for the existing sandboxes | ||
| if err := s.reconcileSandboxConfigMaps(ctx); err != nil { |
There was a problem hiding this comment.
I wonder if there will be race conditions by creating the configMap before the actual sandbox command. Once the ConfigMap is created, the Sandbox reconciler can start to act on the pods. What is preventing them from attempting to restart the pods while we are in the middle of sandboxing them. I think this needs to happen after the sandbox command.
There was a problem hiding this comment.
Here we just create config maps for the sandboxes that already have subclusters. This is for the case we sandbox a subcluster successfully but create a config map failed. Then the reconciler will fix the config map here. I have the code to create the configMap after the actual sandbox command as well.
There was a problem hiding this comment.
Ah ok, yes I see that. You can disregard this comment then.
| // if so, we will update the content of that config map and return true | ||
| func (s *SandboxSubclusterReconciler) updateSandboxConfigMapFields(curCM, newCM *corev1.ConfigMap) bool { | ||
| updated := false | ||
| if stringMapDiffer(curCM.ObjectMeta.Annotations, newCM.ObjectMeta.Annotations) { |
There was a problem hiding this comment.
Roy's PR (#785) is going to add an annotation that the vdb controller will use to wake up the sandbox controller. So, we may need to relax this. Or maybe compare everything except a specific annotation. fyi @roypaulin
There was a problem hiding this comment.
Will that annotation be modified by sandbox controller? In which case that annotation will be changed?
There was a problem hiding this comment.
The vdb controller will modify it to wake up the sandbox controller. It sets a new uuid anytime it needs the sandbox controller to do something. I don't think the sandbox controller will modify it at all.
cchen-vertica
force-pushed
the
cchen/config-map-reconciler
branch
from
97c4f59 to
83aa394
Compare
| // exclude sandbox controller trigger ID from the annotations because | ||
| // vdb controller will set this in current config map, and the new | ||
| // config map cannot get it | ||
| triggerID, ok := curCM.Annotations[vmeta.SandboxControllerTriggerID] |
There was a problem hiding this comment.
We should also exclude the version (vmeta.VersionAnnotation). That can be set in the ConfigMap too.
There was a problem hiding this comment.
Is the version value same as vdb's "vertica.com/version"? If so, the new config map will have the correct version annotation too, and we don't need to exclude the version.
There was a problem hiding this comment.
The values could differ. We will be able to upgrade the sandbox independently from the main cluster.
This will create/update a sandbox config map whenever we create a new sandbox in sandbox reconciler. The config map is watched by sandbox controller for restarting/upgrading the nodes in a sandbox. The config map will be deleted if vdb is deleted or the sandbox is unsandboxed.
This will create/update a sandbox config map whenever we create a new sandbox in sandbox reconciler. The config map is watched by sandbox controller for restarting/upgrading the nodes in a sandbox. The config map will be deleted if vdb is deleted or the sandbox is unsandboxed.
This will create/update a sandbox config map whenever we create a new sandbox in sandbox reconciler. The config map is watched by sandbox controller for restarting/upgrading the nodes in a sandbox. The config map will be deleted if vdb is deleted or the sandbox is unsandboxed.
This will create/update a sandbox config map whenever we create a new sandbox in sandbox reconciler. The config map is watched by sandbox controller for restarting/upgrading the nodes in a sandbox. The config map will be deleted if vdb is deleted or the sandbox is unsandboxed.
This will create/update a sandbox config map whenever we create a new sandbox in sandbox reconciler. The config map is watched by sandbox controller for restarting/upgrading the nodes in a sandbox. The config map will be deleted if vdb is deleted or the sandbox is unsandboxed.
This will create/update a sandbox config map whenever we create a new sandbox in sandbox reconciler. The config map is watched by sandbox controller for restarting/upgrading the nodes in a sandbox. The config map will be deleted if vdb is deleted or the sandbox is unsandboxed.