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

[TT-12626] create k8s development testing guideline for gw/dash #6518

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

andrei-tyk
Copy link
Contributor

@andrei-tyk andrei-tyk commented Sep 16, 2024

User description

https://tyktech.atlassian.net/browse/TT-12626

Description

Added some scripts and makefile targets to allow for easier deployment of Tyk in K8S for development and testing purposes.

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

  • I ensured that the documentation is up to date
  • I explained why this PR updates go.mod in detail with reasoning why it's required
  • I would like a code coverage CI quality gate exception and have explained why

PR Type

enhancement, documentation


Description

  • Enhanced the Makefile to support Kubernetes kind cluster management with new targets for creating and deleting clusters.
  • Added a comprehensive k8sREADME.md file with guidelines for setting up a Kubernetes development environment, including tool installations and cluster management.
  • Introduced a kind.yaml configuration file to define the structure and port mappings of a kind cluster.

Changes walkthrough 📝

Relevant files
Enhancement
Makefile
Add Kubernetes kind cluster management to Makefile             

Makefile

  • Added CLUSTER_NAME variable for kind cluster naming.
  • Introduced create-kind-cluster and delete-kind-cluster targets.
  • Enhanced Makefile for Kubernetes development workflow.
  • +11/-0   
    Documentation
    k8sREADME.md
    Add Kubernetes development and testing guidelines               

    k8s/k8sREADME.md

  • Added installation instructions for Kubernetes tools.
  • Provided steps for creating and deleting a kind cluster.
  • Included instructions for managing Helm values.
  • Documented namespace creation and Redis installation.
  • +55/-0   
    Configuration changes
    kind.yaml
    Define kind cluster configuration for Kubernetes                 

    k8s/kind.yaml

  • Defined a kind cluster configuration with port mappings.
  • Configured roles for control-plane and worker nodes.
  • +11/-0   

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link
    Contributor

    github-actions bot commented Sep 16, 2024

    API Changes

    no api changes detected

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Possible Bug
    The 'clean' target is redefined to delete the kind cluster. This could unintentionally replace the standard 'clean' behavior which usually cleans build artifacts.

    Incomplete Documentation
    The documentation for installing required dependencies in the kind cluster is incomplete. The section on installing Redis ends abruptly without providing the necessary commands.

    Copy link
    Contributor

    github-actions bot commented Sep 16, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Complete the Redis installation command

    The section on installing Redis is incomplete. It's important to provide the full
    command needed to install Redis in the kind cluster to avoid confusion.

    k8s/k8sREADME.md [53]

     #### - Redis
    +```bash
    +helm install redis bitnami/redis --namespace tyk
    +```
     
    Suggestion importance[1-10]: 9

    Why: Completing the Redis installation command is important for providing clear and complete instructions, preventing user confusion and errors.

    9
    Maintainability
    Rename the 'clean' target to 'delete-cluster' for clarity

    The target 'clean' is typically used for removing files that were created by the
    build process. Using 'clean' to delete a kind cluster might be misleading. Consider
    renaming this target to something more descriptive like 'delete-cluster' or
    'clean-cluster'.

    Makefile [104-105]

    -clean:	## Delete kind cluster
    +delete-cluster:	## Delete kind cluster
     kind delete cluster --name=${CLUSTER_NAME}
     
    Suggestion importance[1-10]: 8

    Why: Renaming the 'clean' target to 'delete-cluster' enhances clarity and maintainability, reducing potential confusion about its purpose.

    8
    Best practice
    Replace hardcoded default values with configurable options

    It's a good practice to avoid hardcoding values directly in the Makefile. Instead,
    consider using environment variables or configuration files that can be sourced.
    This makes the Makefile more flexible and easier to maintain.

    Makefile [22]

    -CLUSTER_NAME ?= kind
    +include config.env
    +CLUSTER_NAME ?= $(DEFAULT_CLUSTER_NAME)
     
    Suggestion importance[1-10]: 7

    Why: The suggestion to replace hardcoded values with configurable options improves flexibility and maintainability, but it is not crucial for functionality.

    7
    Add error handling to installation commands

    The installation instructions for 'helm' and 'kind' are missing error handling.
    Consider adding checks to ensure that the commands were successful before
    proceeding.

    k8s/k8sREADME.md [10-17]

    -$ curl -fsSL -o get_helm.sh https://raw.githubusercontent.com/helm/helm/main/scripts/get-helm-3
    -$ chmod 700 get_helm.sh
    -$ ./get_helm.sh
    -go install sigs.k8s.io/[email protected]
    +$ curl -fsSL -o get_helm.sh https://raw.githubusercontent.com/helm/helm/main/scripts/get-helm-3 && echo "Downloaded Helm script successfully" || echo "Failed to download Helm script"
    +$ chmod 700 get_helm.sh && echo "Permission set successfully" || echo "Failed to set permission"
    +$ ./get_helm.sh && echo "Helm installed successfully" || echo "Failed to install Helm"
    +go install sigs.k8s.io/[email protected] && echo "Kind installed successfully" || echo "Failed to install Kind"
     
    Suggestion importance[1-10]: 6

    Why: Adding error handling to installation commands is a good practice for robustness, but it is not critical for the basic functionality of the instructions.

    6

    Copy link

    @titpetric titpetric changed the title Tt 12626 create k 8 s development testing guideline for gw dash [TT-12626] create k8s development testing guideline for gw/dash Sep 18, 2024
    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