Skip to content

Conversation

@rswanson
Copy link
Member

No description provided.

@rswanson rswanson self-assigned this May 20, 2025
This was referenced May 20, 2025
@rswanson rswanson marked this pull request as ready for review May 20, 2025 21:09
@rswanson rswanson requested a review from Copilot May 20, 2025 21:10
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements consistent labeling across Kubernetes resources by centralizing label creation in a single helper and refactoring resource definitions to use it.

  • Moved and updated CreateResourceLabels to pkg/utils/labels.go and its tests.
  • Refactored resource creation (PVCs, ConfigMaps, Deployments, Services, StatefulSets) to use consistent labeling and helper functions.
  • Updated naming conventions across the signet_node and builder components for clarity and consistency.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
pkg/utils/labels_test.go Added tests to verify consistent label creation.
pkg/utils/labels.go Introduced centralized label creation with merging additional labels.
pkg/signet_node/signet_node.go Refactored resource creations to leverage helper functions for labels.
pkg/signet_node/helpers_test.go Updated tests to reflect new label location.
pkg/signet_node/helpers.go Removed obsolete CreateResourceLabels function.
pkg/builder/helpers.go Updated to call centralized label helper.
pkg/builder/builder.go Refactored resource names and labels for service accounts, deployments, and services.
Comments suppressed due to low confidence (2)

pkg/signet_node/signet_node.go:69

  • The config map name 'exex-configmap' appears to be misspelled. Consider renaming it to 'exec-configmap' or 'execution-configmap' for clarity and consistency.
executionConfigMapName := "exex-configmap"

pkg/signet_node/signet_node.go:139

  • [nitpick] Appending '-set' to hostStatefulSetName (which is already 'signet-node-execution-set') leads to a redundant resource name ('signet-node-execution-set-set'). Verify if this naming is intentional or adjust to avoid duplication.
hostStatefulSetResourceName := fmt.Sprintf("%s-set", hostStatefulSetName)

@rswanson rswanson mentioned this pull request May 27, 2025
@rswanson rswanson force-pushed the swanny/consistent-labeling branch from 57f5e7d to 70c5d1d Compare May 27, 2025 16:35
@rswanson rswanson force-pushed the swanny/aws-for-now branch from 55df81a to e4b185d Compare May 27, 2025 16:56
@rswanson rswanson force-pushed the swanny/consistent-labeling branch from 70c5d1d to d4b0242 Compare May 27, 2025 16:56
@rswanson rswanson requested review from Evalir and dylanlott May 27, 2025 17:00
@rswanson rswanson force-pushed the swanny/consistent-labeling branch from d4b0242 to 5b2886c Compare May 27, 2025 17:06
@rswanson rswanson force-pushed the swanny/aws-for-now branch from e4b185d to 35cfcf6 Compare May 27, 2025 17:06
This was referenced May 27, 2025
@rswanson rswanson force-pushed the swanny/aws-for-now branch from 35cfcf6 to 8430abf Compare May 29, 2025 11:25
@rswanson rswanson force-pushed the swanny/consistent-labeling branch 2 times, most recently from cfa82fa to a76a2c1 Compare May 29, 2025 11:29
@rswanson rswanson force-pushed the swanny/aws-for-now branch from 8430abf to 477f988 Compare May 29, 2025 11:29
@rswanson rswanson force-pushed the swanny/aws-for-now branch 2 times, most recently from ad1e1fc to 1d0d165 Compare May 29, 2025 13:53
@rswanson rswanson force-pushed the swanny/consistent-labeling branch 2 times, most recently from 9c043f0 to 4a5e5b2 Compare May 29, 2025 13:55
@rswanson rswanson changed the base branch from swanny/aws-for-now to graphite-base/5 May 29, 2025 15:22
@rswanson rswanson force-pushed the swanny/consistent-labeling branch from 4a5e5b2 to 4c6b6ea Compare May 29, 2025 15:22
@graphite-app graphite-app bot changed the base branch from graphite-base/5 to main May 29, 2025 15:23
@rswanson rswanson force-pushed the swanny/consistent-labeling branch from 4c6b6ea to 7d779ad Compare May 29, 2025 15:23
@rswanson rswanson merged commit b94d3f6 into main May 29, 2025
2 checks passed
Copy link
Member Author

Merge activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants