Skip to content

Commit a6da3f6

Browse files
Address code review feedback - extract constants and shared methods
Co-authored-by: brendandburns <[email protected]>
1 parent b814eaa commit a6da3f6

File tree

2 files changed

+27
-18
lines changed

2 files changed

+27
-18
lines changed

src/KubernetesClient.Kubectl/Beta/AsyncKubectl.Rollout.cs

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,15 @@ namespace k8s.kubectl.beta;
66

77
public partial class AsyncKubectl
88
{
9+
private const string RestartedAtAnnotation = "kubectl.kubernetes.io/restartedAt";
10+
private const string RevisionAnnotation = "deployment.kubernetes.io/revision";
11+
private const string ChangeCauseAnnotation = "kubernetes.io/change-cause";
12+
13+
private static string BuildLabelSelector(IDictionary<string, string> matchLabels)
14+
{
15+
return string.Join(",", matchLabels.Select(kvp => $"{kvp.Key}={kvp.Value}"));
16+
}
17+
918
/// <summary>
1019
/// Restart a Deployment by adding a restart annotation to trigger a rollout.
1120
/// </summary>
@@ -22,7 +31,7 @@ public async Task RolloutRestartDeploymentAsync(string name, string @namespace,
2231
// Add or update the restart annotation to trigger a rollout
2332
deployment.Spec.Template.Metadata ??= new V1ObjectMeta();
2433
deployment.Spec.Template.Metadata.Annotations ??= new Dictionary<string, string>();
25-
deployment.Spec.Template.Metadata.Annotations["kubectl.kubernetes.io/restartedAt"] = DateTime.UtcNow.ToString("o");
34+
deployment.Spec.Template.Metadata.Annotations[RestartedAtAnnotation] = DateTime.UtcNow.ToString("o");
2635

2736
var patch = old.CreatePatch(deployment);
2837

@@ -45,7 +54,7 @@ public async Task RolloutRestartDaemonSetAsync(string name, string @namespace, C
4554
// Add or update the restart annotation to trigger a rollout
4655
daemonSet.Spec.Template.Metadata ??= new V1ObjectMeta();
4756
daemonSet.Spec.Template.Metadata.Annotations ??= new Dictionary<string, string>();
48-
daemonSet.Spec.Template.Metadata.Annotations["kubectl.kubernetes.io/restartedAt"] = DateTime.UtcNow.ToString("o");
57+
daemonSet.Spec.Template.Metadata.Annotations[RestartedAtAnnotation] = DateTime.UtcNow.ToString("o");
4958

5059
var patch = old.CreatePatch(daemonSet);
5160

@@ -68,7 +77,7 @@ public async Task RolloutRestartStatefulSetAsync(string name, string @namespace,
6877
// Add or update the restart annotation to trigger a rollout
6978
statefulSet.Spec.Template.Metadata ??= new V1ObjectMeta();
7079
statefulSet.Spec.Template.Metadata.Annotations ??= new Dictionary<string, string>();
71-
statefulSet.Spec.Template.Metadata.Annotations["kubectl.kubernetes.io/restartedAt"] = DateTime.UtcNow.ToString("o");
80+
statefulSet.Spec.Template.Metadata.Annotations[RestartedAtAnnotation] = DateTime.UtcNow.ToString("o");
7281

7382
var patch = old.CreatePatch(statefulSet);
7483

@@ -254,15 +263,15 @@ public async Task RolloutUndoDeploymentAsync(string name, string @namespace, lon
254263
var deployment = await client.AppsV1.ReadNamespacedDeploymentAsync(name, @namespace, cancellationToken: cancellationToken).ConfigureAwait(false);
255264

256265
// Get all ReplicaSets for this deployment
257-
var labelSelector = string.Join(",", deployment.Spec.Selector.MatchLabels.Select(kvp => $"{kvp.Key}={kvp.Value}"));
266+
var labelSelector = BuildLabelSelector(deployment.Spec.Selector.MatchLabels);
258267
var replicaSets = await client.AppsV1.ListNamespacedReplicaSetAsync(@namespace, labelSelector: labelSelector, cancellationToken: cancellationToken).ConfigureAwait(false);
259268

260269
// Filter ReplicaSets owned by this deployment
261270
var ownedReplicaSets = replicaSets.Items
262271
.Where(rs => rs.Metadata.OwnerReferences?.Any(or => or.Uid == deployment.Metadata.Uid) == true)
263272
.OrderByDescending(rs =>
264273
{
265-
if (rs.Metadata.Annotations?.TryGetValue("deployment.kubernetes.io/revision", out var revisionStr) == true)
274+
if (rs.Metadata.Annotations?.TryGetValue(RevisionAnnotation, out var revisionStr) == true)
266275
{
267276
return long.TryParse(revisionStr, out var revision) ? revision : 0;
268277
}
@@ -283,7 +292,7 @@ public async Task RolloutUndoDeploymentAsync(string name, string @namespace, lon
283292
// Find specific revision
284293
targetReplicaSet = ownedReplicaSets.FirstOrDefault(rs =>
285294
{
286-
if (rs.Metadata.Annotations?.TryGetValue("deployment.kubernetes.io/revision", out var revisionStr) == true)
295+
if (rs.Metadata.Annotations?.TryGetValue(RevisionAnnotation, out var revisionStr) == true)
287296
{
288297
return long.TryParse(revisionStr, out var revision) && revision == toRevision.Value;
289298
}
@@ -314,8 +323,8 @@ public async Task RolloutUndoDeploymentAsync(string name, string @namespace, lon
314323

315324
// Add annotation to record the rollback
316325
deployment.Metadata.Annotations ??= new Dictionary<string, string>();
317-
deployment.Metadata.Annotations["deployment.kubernetes.io/revision"] =
318-
targetReplicaSet.Metadata.Annotations?["deployment.kubernetes.io/revision"] ?? "0";
326+
deployment.Metadata.Annotations[RevisionAnnotation] =
327+
targetReplicaSet.Metadata.Annotations?[RevisionAnnotation] ?? "0";
319328

320329
var patch = old.CreatePatch(deployment);
321330

@@ -334,7 +343,7 @@ public async Task<IList<RolloutHistoryEntry>> RolloutHistoryDeploymentAsync(stri
334343
var deployment = await client.AppsV1.ReadNamespacedDeploymentAsync(name, @namespace, cancellationToken: cancellationToken).ConfigureAwait(false);
335344

336345
// Get all ReplicaSets for this deployment
337-
var labelSelector = string.Join(",", deployment.Spec.Selector.MatchLabels.Select(kvp => $"{kvp.Key}={kvp.Value}"));
346+
var labelSelector = BuildLabelSelector(deployment.Spec.Selector.MatchLabels);
338347
var replicaSets = await client.AppsV1.ListNamespacedReplicaSetAsync(@namespace, labelSelector: labelSelector, cancellationToken: cancellationToken).ConfigureAwait(false);
339348

340349
// Filter and process ReplicaSets owned by this deployment
@@ -343,13 +352,13 @@ public async Task<IList<RolloutHistoryEntry>> RolloutHistoryDeploymentAsync(stri
343352
.Select(rs =>
344353
{
345354
var revision = 0L;
346-
if (rs.Metadata.Annotations?.TryGetValue("deployment.kubernetes.io/revision", out var revisionStr) == true)
355+
if (rs.Metadata.Annotations?.TryGetValue(RevisionAnnotation, out var revisionStr) == true)
347356
{
348357
long.TryParse(revisionStr, out revision);
349358
}
350359

351360
var changeCause = "<none>";
352-
if (rs.Metadata.Annotations?.TryGetValue("kubernetes.io/change-cause", out var cause) == true && !string.IsNullOrEmpty(cause))
361+
if (rs.Metadata.Annotations?.TryGetValue(ChangeCauseAnnotation, out var cause) == true && !string.IsNullOrEmpty(cause))
353362
{
354363
changeCause = cause;
355364
}
@@ -379,7 +388,7 @@ public async Task<IList<RolloutHistoryEntry>> RolloutHistoryDaemonSetAsync(strin
379388
var daemonSet = await client.AppsV1.ReadNamespacedDaemonSetAsync(name, @namespace, cancellationToken: cancellationToken).ConfigureAwait(false);
380389

381390
// Get ControllerRevisions for this DaemonSet
382-
var labelSelector = string.Join(",", daemonSet.Spec.Selector.MatchLabels.Select(kvp => $"{kvp.Key}={kvp.Value}"));
391+
var labelSelector = BuildLabelSelector(daemonSet.Spec.Selector.MatchLabels);
383392
var controllerRevisions = await client.AppsV1.ListNamespacedControllerRevisionAsync(@namespace, labelSelector: labelSelector, cancellationToken: cancellationToken).ConfigureAwait(false);
384393

385394
// Filter and process ControllerRevisions owned by this DaemonSet
@@ -388,7 +397,7 @@ public async Task<IList<RolloutHistoryEntry>> RolloutHistoryDaemonSetAsync(strin
388397
.Select(cr =>
389398
{
390399
var changeCause = "<none>";
391-
if (cr.Metadata.Annotations?.TryGetValue("kubernetes.io/change-cause", out var cause) == true && !string.IsNullOrEmpty(cause))
400+
if (cr.Metadata.Annotations?.TryGetValue(ChangeCauseAnnotation, out var cause) == true && !string.IsNullOrEmpty(cause))
392401
{
393402
changeCause = cause;
394403
}
@@ -417,7 +426,7 @@ public async Task<IList<RolloutHistoryEntry>> RolloutHistoryStatefulSetAsync(str
417426
var statefulSet = await client.AppsV1.ReadNamespacedStatefulSetAsync(name, @namespace, cancellationToken: cancellationToken).ConfigureAwait(false);
418427

419428
// Get ControllerRevisions for this StatefulSet
420-
var labelSelector = string.Join(",", statefulSet.Spec.Selector.MatchLabels.Select(kvp => $"{kvp.Key}={kvp.Value}"));
429+
var labelSelector = BuildLabelSelector(statefulSet.Spec.Selector.MatchLabels);
421430
var controllerRevisions = await client.AppsV1.ListNamespacedControllerRevisionAsync(@namespace, labelSelector: labelSelector, cancellationToken: cancellationToken).ConfigureAwait(false);
422431

423432
// Filter and process ControllerRevisions owned by this StatefulSet
@@ -426,7 +435,7 @@ public async Task<IList<RolloutHistoryEntry>> RolloutHistoryStatefulSetAsync(str
426435
.Select(cr =>
427436
{
428437
var changeCause = "<none>";
429-
if (cr.Metadata.Annotations?.TryGetValue("kubernetes.io/change-cause", out var cause) == true && !string.IsNullOrEmpty(cause))
438+
if (cr.Metadata.Annotations?.TryGetValue(ChangeCauseAnnotation, out var cause) == true && !string.IsNullOrEmpty(cause))
430439
{
431440
changeCause = cause;
432441
}

tests/Kubectl.Tests/KubectlTests.Rollout.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public void RolloutRestartDeployment()
7171
// Verify the restart annotation was added
7272
var updatedDeployment = kubernetes.AppsV1.ReadNamespacedDeployment(deploymentName, namespaceParameter);
7373
Assert.NotNull(updatedDeployment.Spec.Template.Metadata.Annotations);
74-
Assert.True(updatedDeployment.Spec.Template.Metadata.Annotations.ContainsKey("kubectl.kubernetes.io/restartedAt"));
74+
Assert.Contains("kubectl.kubernetes.io/restartedAt", updatedDeployment.Spec.Template.Metadata.Annotations.Keys);
7575
}
7676
finally
7777
{
@@ -394,7 +394,7 @@ public void RolloutRestartDaemonSet()
394394
// Verify the restart annotation was added
395395
var updatedDaemonSet = kubernetes.AppsV1.ReadNamespacedDaemonSet(daemonSetName, namespaceParameter);
396396
Assert.NotNull(updatedDaemonSet.Spec.Template.Metadata.Annotations);
397-
Assert.True(updatedDaemonSet.Spec.Template.Metadata.Annotations.ContainsKey("kubectl.kubernetes.io/restartedAt"));
397+
Assert.Contains("kubectl.kubernetes.io/restartedAt", updatedDaemonSet.Spec.Template.Metadata.Annotations.Keys);
398398
}
399399
finally
400400
{
@@ -554,7 +554,7 @@ public void RolloutRestartStatefulSet()
554554
// Verify the restart annotation was added
555555
var updatedStatefulSet = kubernetes.AppsV1.ReadNamespacedStatefulSet(statefulSetName, namespaceParameter);
556556
Assert.NotNull(updatedStatefulSet.Spec.Template.Metadata.Annotations);
557-
Assert.True(updatedStatefulSet.Spec.Template.Metadata.Annotations.ContainsKey("kubectl.kubernetes.io/restartedAt"));
557+
Assert.Contains("kubectl.kubernetes.io/restartedAt", updatedStatefulSet.Spec.Template.Metadata.Annotations.Keys);
558558
}
559559
finally
560560
{

0 commit comments

Comments
 (0)