Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refs platform/3278: Add option for gitaly requests #85

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

Syphon83
Copy link

@Syphon83 Syphon83 commented Nov 26, 2024

PR Type

Enhancement


Description

  • Added configuration options to specify CPU and memory resource requests for Gitaly pods
  • Set default values of 100m CPU and 200Mi memory for Gitaly pods
  • Implemented the resource request configuration in the Gitlab Helm values

Changes walkthrough 📝

Relevant files
Configuration changes
main.tf
Add Gitaly resource request variables to locals                   

main.tf

  • Added new variables GITALY_REQUEST_CPU and GITALY_REQUEST_MEMORY to
    locals block
  • +2/-0     
    variables.tf
    Define Gitaly CPU and memory request variables                     

    variables.tf

  • Added new variable gitlab_gitaly_request_cpu for CPU requests with
    default 100m
  • Added new variable gitlab_gitaly_request_mem for memory requests with
    default 200Mi
  • +12/-0   
    values.yaml
    Configure Gitaly pod resource requests                                     

    values.yaml

  • Added resource requests configuration for Gitaly pods
  • Configured CPU and memory requests using the new variables
  • +4/-0     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @sparkfabrik-ai-bot
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Resource Validation
    The CPU and memory request variables should have validation constraints to ensure valid values are provided, as invalid resource specifications could cause pod scheduling failures

    Resource Limits
    Only resource requests are specified without corresponding resource limits, which could allow pods to consume excessive resources

    @sparkfabrik-ai-bot
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Add input validation to ensure resource requests follow the correct format

    Add validation rules to ensure CPU and memory request values follow Kubernetes
    resource format (e.g., for CPU: "100m" or "0.1", for memory: "200Mi" or "1Gi").

    variables.tf [794-798]

     variable "gitlab_gitaly_request_cpu" {
       type        = string
       description = "CPU request for gitaly POD. Measurement unit needs to be specified. Default 100m."
       default     = "100m"
    +  validation {
    +    condition     = can(regex("^[0-9]+(m|\\.[0-9]+)$", var.gitlab_gitaly_request_cpu))
    +    error_message = "CPU request must be a valid Kubernetes CPU value (e.g., 100m or 0.1)."
    +  }
     }
    Suggestion importance[1-10]: 8

    Why: Adding validation rules for CPU request format is crucial for preventing deployment failures due to invalid resource specifications. The suggestion provides precise regex validation that matches Kubernetes CPU format requirements.

    8
    Validate memory request values to prevent configuration errors

    Add validation rule to ensure memory request values are positive and follow
    Kubernetes memory format.

    variables.tf [800-804]

     variable "gitlab_gitaly_request_mem" {
       type        = string
       description = "Memory request for gitaly POD. Measurement unit needs to be specified. Default 200Mi."
       default     = "200Mi"
    +  validation {
    +    condition     = can(regex("^[0-9]+(Mi|Gi|Ti|Ki)$", var.gitlab_gitaly_request_mem))
    +    error_message = "Memory request must be a valid Kubernetes memory value (e.g., 200Mi, 1Gi)."
    +  }
     }
    Suggestion importance[1-10]: 8

    Why: The validation rule for memory requests is essential to ensure valid Kubernetes memory units are used, preventing deployment issues. The regex pattern correctly validates all valid Kubernetes memory units.

    8
    Enhancement
    Add resource limits to prevent container resource overuse

    Consider adding resource limits alongside requests to prevent potential resource
    exhaustion by Gitaly pods.

    values.yaml [289-292]

     resources:
       requests:
         cpu: ${GITALY_REQUEST_CPU}
         memory: ${GITALY_REQUEST_MEMORY}
    +  limits:
    +    cpu: ${GITALY_LIMIT_CPU}
    +    memory: ${GITALY_LIMIT_MEMORY}
    Suggestion importance[1-10]: 7

    Why: Adding resource limits is a good practice to prevent resource exhaustion and ensure proper pod scheduling, though it requires careful consideration of appropriate limit values to avoid pod throttling.

    7

    @Syphon83 Syphon83 merged commit a82050a into master Nov 26, 2024
    1 check passed
    @Syphon83 Syphon83 deleted the gitaly_resources branch November 26, 2024 11:51
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant