mirror of
https://github.com/cloudnative-pg/plugin-barman-cloud.git
synced 2026-09-04 14:12:21 +02:00
style: tighten comments
Trim redundant/restating comments and cut wording in the shouldInjectBarmanSidecar doc-comment without dropping any of the non-obvious reasoning it documents. Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
This commit is contained in:
parent
fb3397386c
commit
0689c451c1
@ -39,8 +39,7 @@ var _ = Describe("IdentityImplementation", func() {
|
||||
serviceTypes = append(serviceTypes, capability.GetService().GetType())
|
||||
}
|
||||
|
||||
// The instance sidecar now runs the phase-0 restore in-process, so it must
|
||||
// advertise TYPE_RESTORE_JOB alongside the services it already served.
|
||||
// Runs the phase-0 restore in-process now, hence TYPE_RESTORE_JOB below.
|
||||
Expect(serviceTypes).To(ConsistOf(
|
||||
identity.PluginCapability_Service_TYPE_WAL_SERVICE,
|
||||
identity.PluginCapability_Service_TYPE_BACKUP_SERVICE,
|
||||
|
||||
@ -102,9 +102,7 @@ func (config *PluginConfiguration) GetReplicaSourceBarmanObjectKey() types.Names
|
||||
}
|
||||
}
|
||||
|
||||
// HasAnyBarmanObjectStore returns true if the configuration references at least
|
||||
// one barman object store, be it for backup/archiving, recovery, or as a
|
||||
// replica source.
|
||||
// HasAnyBarmanObjectStore reports whether any barman object store is configured.
|
||||
func (config *PluginConfiguration) HasAnyBarmanObjectStore() bool {
|
||||
return len(config.BarmanObjectName) > 0 ||
|
||||
len(config.RecoveryBarmanObjectName) > 0 ||
|
||||
|
||||
@ -325,27 +325,24 @@ func (impl LifecycleImplementation) collectAdditionalInstanceArgs(
|
||||
// shouldInjectBarmanSidecar decides whether an instance pod needs the
|
||||
// plugin-barman-cloud sidecar.
|
||||
//
|
||||
// A cluster doing backup/archiving or serving as a replica source needs the
|
||||
// sidecar in every instance pod for as long as the cluster exists, so those
|
||||
// two cases always inject it. A recovery-only cluster (only
|
||||
// RecoveryBarmanObjectName set, mirroring what pluginConfiguration.Validate()
|
||||
// accepts) only ever needs the sidecar for its one-time bootstrap restore, so
|
||||
// it's gated on cluster.Status.CurrentPrimary instead.
|
||||
// Backup/archiving and replica-source configs need it for as long as the
|
||||
// cluster exists, so those always inject it. A recovery-only cluster (only
|
||||
// RecoveryBarmanObjectName set, mirroring pluginConfiguration.Validate())
|
||||
// only needs it for the one-time bootstrap restore, so it's gated on
|
||||
// cluster.Status.CurrentPrimary instead.
|
||||
//
|
||||
// CurrentPrimary is set by the instance manager itself, from inside the
|
||||
// primary pod, only once it's up and has completed its own bootstrap (see
|
||||
// instance_startup.go in cloudnative-pg) - unlike cluster.Status.Instances /
|
||||
// IsInitialized(), which flips as soon as the instance's PVC exists, well
|
||||
// before the pod is even created. Using IsInitialized() here would mean the
|
||||
// sidecar is never added to the one pod that needs it to perform its restore.
|
||||
// CurrentPrimary is set by the instance manager itself, from inside the pod,
|
||||
// only once bootstrap completes (see instance_startup.go in cloudnative-pg).
|
||||
// cluster.Status.Instances / IsInitialized() looks equivalent but flips as
|
||||
// soon as the instance's PVC exists, before the pod is even created - using
|
||||
// it here would mean the sidecar never reaches the pod that needs it.
|
||||
//
|
||||
// Once CurrentPrimary is set, this makes the operator's own drift-check
|
||||
// (checkPodSpecIsOutdated) see the running pod's spec as outdated and roll it
|
||||
// out to drop the sidecar. That's deliberately accepted rather than
|
||||
// engineered around: it's one deterministic rollout using the same machinery
|
||||
// the operator already uses for every other pod-spec change (a switchover if
|
||||
// a replica is available, an in-place restart otherwise), not a new or
|
||||
// fragile risk.
|
||||
// Once CurrentPrimary is set, the operator's drift-check
|
||||
// (checkPodSpecIsOutdated) sees the running pod's spec as outdated and rolls
|
||||
// it out to drop the sidecar. Accepted deliberately: one deterministic
|
||||
// rollout via the same machinery used for any other pod-spec change
|
||||
// (switchover if a replica exists, in-place restart otherwise), not a new
|
||||
// risk.
|
||||
func shouldInjectBarmanSidecar(
|
||||
cluster *cnpgv1.Cluster,
|
||||
pluginConfiguration *config.PluginConfiguration,
|
||||
|
||||
@ -302,7 +302,6 @@ var _ = Describe("LifecycleImplementation", func() {
|
||||
response, err := reconcileInstancePod(ctx, cluster, request, emptyConfig, sidecarConfiguration{})
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
Expect(response).NotTo(BeNil())
|
||||
// An empty patch means the pod was left untouched: no sidecar injected.
|
||||
Expect(response.JsonPatch).To(BeEmpty())
|
||||
})
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user