Closed DaMandal0rian closed 1 month ago
PR Description updated to latest commit (https://github.com/subspace/infra/commit/e2cae668418131bd4db2df543de00247e1602838)
⏱️ Estimated effort to review [1-5] | 4, due to the extensive changes across multiple Kubernetes configuration files, including ConfigMaps, Services, StatefulSets, and StorageClasses. The PR involves complex configurations that require careful review to ensure they meet the intended deployment specifications and security standards. |
🧪 Relevant tests | No |
🔍 Possible issues | Possible Bug: The service name in the `archival-node-service.yaml` file is misspelled as `boostrap-domain-node-service` instead of `bootstrap-domain-node-service`. This could lead to issues when other resources try to reference this service. |
Configuration Consistency: The `metadata.namespace` for the `domain-node-pvc.yaml` is set to `default`, which might not be consistent with other configurations that specify a different namespace, such as `bootstrap-domain`. This inconsistency can lead to deployment issues in a multi-namespace environment. | |
🔒 Security concerns | No |
relevant file | kubernetes/devnet/base/bootstrap-domain-node/archival-node-service.yaml |
suggestion | Correct the typo in the service name from `boostrap-domain-node-service` to `bootstrap-domain-node-service`. This change is important to ensure that other resources can correctly reference this service. [important] |
relevant line | name: boostrap-domain-node-service |
relevant file | kubernetes/devnet/base/domain-node/pvc.yaml |
suggestion | Ensure the namespace for the `domain-node-pvc` matches the intended deployment environment or is consistent with other PVC configurations to avoid deployment issues in a multi-namespace setup. Consider setting it explicitly to a specific namespace used by other domain node resources. [important] |
relevant line | namespace: default # Ensure this is set to the namespace where your workload runs |
Category | Suggestions |
Bug |
Correct the typo in the service name for consistency and correct referencing.___ **Correct the typo in the service name from 'boostrap-domain-node-service' to'bootstrap-domain-node-service' to match the intended naming convention and ensure consistency across configurations.** [kubernetes/devnet/base/bootstrap-domain-node/archival-node-service.yaml [4]](https://github.com/subspace/infra/pull/308/files#diff-426bba379c71565d075374f030fd4551d5fc2284ac912c4e3860cc4ba36de050R4-R4) ```diff -name: boostrap-domain-node-service +name: bootstrap-domain-node-service ``` |
Correct the environment variable placeholder syntax for proper substitution.___ **Replace the placeholder${DSN_NODE_KEY} with the correct syntax $(DSN_NODE_KEY) to ensure proper value substitution from the environment variables in Kubernetes.** [kubernetes/devnet/base/bootstrap-domain-node/archival-node.yaml [128]](https://github.com/subspace/infra/pull/308/files#diff-8f406fa9a22d59707564e2982062351968fd974319d584bd46ffa76d112975a1R128-R128) ```diff -- ${DSN_NODE_KEY} +- $(DSN_NODE_KEY) ``` | |
Enhancement |
Specify
___
**Ensure that the |
Best practice |
Define resource limits for the container to manage resource usage effectively.___ **Add resource limits for the container 'dsn-bootstrap-node' to prevent potential resourceexhaustion, ensuring better resource management and stability.** [kubernetes/devnet/base/bootstrap-domain-node/archival-node.yaml [184-186]](https://github.com/subspace/infra/pull/308/files#diff-8f406fa9a22d59707564e2982062351968fd974319d584bd46ffa76d112975a1R184-R186) ```diff resources: requests: cpu: "500m" memory: "1Gi" + limits: + cpu: "1" + memory: "2Gi" ``` |
Change the namespace to a non-default one for better resource management.___ **Consider specifying a more appropriate namespace instead of using the default namespacefor better isolation and management of Kubernetes resources.** [kubernetes/devnet/base/domain-node/ingress.yaml [5]](https://github.com/subspace/infra/pull/308/files#diff-ec1b84f6931fd043490fc82c069fed9898de3233a036faa4787f034a06dbe5f2R5-R5) ```diff -namespace: default +namespace: devnet ``` | |
Add explicit
___
**For the service ports that do not specify a | |
Maintainability |
Remove duplicate command-line arguments to avoid configuration errors.___ **Remove redundant command-line arguments related to state and block pruning since they arespecified multiple times, which could lead to confusion and misconfiguration.** [kubernetes/devnet/base/bootstrap-domain-node/archival-node.yaml [253-255]](https://github.com/subspace/infra/pull/308/files#diff-8f406fa9a22d59707564e2982062351968fd974319d584bd46ffa76d112975a1R253-R255) ```diff - "--state-pruning" - "archive" -- "--blocks-pruning" -- "archive" ``` |
Clarify or remove the namespace comment for better code clarity.___ **It's recommended to specify a more descriptive comment regarding the namespace requirementor to remove the comment if the namespace is self-explanatory.** [kubernetes/devnet/base/domain-node/pvc.yaml [5]](https://github.com/subspace/infra/pull/308/files#diff-4349b7aa8b378520c50fd22a4bcadbdb05cbbcd54896b45783d8a54a626dad40R5-R5) ```diff -namespace: default # Ensure this is set to the namespace where your workload runs +namespace: default ``` | |
Verify or document the setup for
___
**Ensure that the | |
Performance |
Remove the
___
**Consider removing the |
Type
enhancement
Description
Changes walkthrough
11 files
archival-node-configmap.yaml
Add ConfigMap for Bootstrap Archival Node
kubernetes/devnet/base/bootstrap-domain-node/archival-node-configmap.yaml
archival-node-service.yaml
Define Service for Bootstrap Domain Nodes
kubernetes/devnet/base/bootstrap-domain-node/archival-node-service.yaml
archival-node.yaml
Setup StatefulSet for Bootstrap Domain Node
kubernetes/devnet/base/bootstrap-domain-node/archival-node.yaml
container specs.
pvc.yaml
Add PVC for Bootstrap Archival Node
kubernetes/devnet/base/bootstrap-domain-node/pvc.yaml
storageclass-aws.yaml
Define AWS EBS StorageClass for Bootstrap Node
kubernetes/devnet/base/bootstrap-domain-node/storageclass-aws.yaml
archival-node-configmap.yaml
Add ConfigMap for Domain Node
kubernetes/devnet/base/domain-node/archival-node-configmap.yaml
archival-node.yaml
Setup StatefulSet for Domain Node
kubernetes/devnet/base/domain-node/archival-node.yaml
ingress.yaml
Configure Ingress for Domain Node
kubernetes/devnet/base/domain-node/ingress.yaml
pvc.yaml
Add PVC for Domain Node
kubernetes/devnet/base/domain-node/pvc.yaml
service.yaml
Define Service for Domain Nodes
kubernetes/devnet/base/domain-node/service.yaml
storageclass-aws.yaml
Define AWS EBS StorageClass for Domain Node
kubernetes/devnet/base/domain-node/storageclass-aws.yaml