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.

Sign up for GitHub

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

Jump to bottom

[k8s]: transition integration tests to adapter pattern #6277

Merged
merged 2 commits into from
Dec 17, 2024
Merged

[k8s]: transition integration tests to adapter pattern #6277

merged 2 commits into from
Dec 17, 2024

Conversation

pkoutsovasilis
Copy link
Contributor

@pkoutsovasilis pkoutsovasilis commented Dec 11, 2024 •
edited
Loading

What does this PR do?

This PR continues the groundwork for Kubernetes hints integration tests. The next and last episode of this PR-series can be seen here 2ea1f2f. Again these were split up because I want to keep each PR around 500 lines.

Key changes in this PR:

  1. Transition Kubernetes integration tests to an "adapter" pattern:

    • Refactors Kubernetes integration tests to follow a more modular and reusable adapter pattern.
    • Introduces k8sTestStep functions that encapsulate individual test steps for clarity and flexibility.
    • Simplifies the test flow by eliminating repetitive code and leveraging reusable functions like k8sStepCreateNamespace, k8sStepDeployKustomize, and k8sStepRunInnerTests.
    • Improves readability and maintainability.

  2. Align all kubernetes integration tests:

    • All kubernetes integration tests got an empty for now skipReason.
    • All kubernetes integration tests check for schedulable k8s nodes and got more deterministic in the number of agent pods expected to be running.

Why is it important?

The adapter pattern for Kubernetes integration tests enhances readability, maintainability, making it easier to add or modify test cases.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • [ ] I have made corresponding changes to the documentation
  • [ ] I have made corresponding changes to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • [ ] I have added an entry in ./changelog/fragments using the changelog tool
  • I have added an integration test or an E2E test

Disruptive User Impact

  • None expected. The changes affect only the internal kubernetes integration tests without altering user-facing configurations or behaviours.

How to test this PR locally

mage integration:auth
PLATFORMS=linux/arm64 EXTERNAL=true SNAPSHOT=true PACKAGES=docker mage -v package 
INSTANCE_PROVISIONER=kind STACK_PROVISIONER=stateful K8S_VERSION=v1.31.1 SNAPSHOT=true mage integration:kubernetes

Related issues

@pkoutsovasilis pkoutsovasilis added Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team skip-changelog Testing backport-8.x Automated backport to the 8.x branch with mergify backport-8.16 Automated backport with mergify backport-8.17 Automated backport with mergify labels Dec 11, 2024
@pkoutsovasilis pkoutsovasilis self-assigned this Dec 11, 2024
@elastic-sonarqube Elastic SonarQube
Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube

@pkoutsovasilis pkoutsovasilis marked this pull request as ready for review December 11, 2024 12:19
@pkoutsovasilis pkoutsovasilis requested a review from a team as a code owner December 11, 2024 12:19
@elasticmachine
Copy link
Contributor

Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane)

@pkoutsovasilis
Copy link
Contributor Author

@blakerouse @swiatekm could you give this PR a review? 🙏

swiatekm
swiatekm reviewed Dec 17, 2024
View reviewed changes
Copy link
Contributor

@swiatekm swiatekm left a comment

Choose a reason for hiding this comment

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

Looks good to me overall, left some comments.

testing/integration/kubernetes_agent_standalone_test.go Show resolved Hide resolved
testing/integration/kubernetes_agent_service_test.go Show resolved Hide resolved
pchila
pchila approved these changes Dec 17, 2024
View reviewed changes
Copy link
Member

@pchila pchila left a comment

Choose a reason for hiding this comment

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

Looks good, test are more readable by refactoring common boilerplate in separate functions.

testing/integration/kubernetes_agent_standalone_test.go Show resolved Hide resolved
@pkoutsovasilis pkoutsovasilis merged commit 67c744f into elastic:main Dec 17, 2024
13 checks passed
@pkoutsovasilis pkoutsovasilis deleted the pkoutsovasilis/k8s_hints_tests_pt2 branch December 17, 2024 17:13
mergify bot pushed a commit that referenced this pull request Dec 17, 2024
@pkoutsovasilis @mergify
[k8s]: transition integration tests to adapter pattern (#6277)
ec16b22 
* feat: transition kubernetes integration tests to adapter pattern

* fix: count nodes without NoSchedule taints

(cherry picked from commit 67c744f)
mergify bot pushed a commit that referenced this pull request Dec 17, 2024
@pkoutsovasilis @mergify
[k8s]: transition integration tests to adapter pattern (#6277)
91ab034 
* feat: transition kubernetes integration tests to adapter pattern

* fix: count nodes without NoSchedule taints

(cherry picked from commit 67c744f)
mergify bot pushed a commit that referenced this pull request Dec 17, 2024
@pkoutsovasilis @mergify
[k8s]: transition integration tests to adapter pattern (#6277)
63f993b 
* feat: transition kubernetes integration tests to adapter pattern

* fix: count nodes without NoSchedule taints

(cherry picked from commit 67c744f)
This was referenced Dec 17, 2024
Merged
Merged
Merged
pkoutsovasilis added a commit that referenced this pull request Dec 18, 2024
@mergify @pkoutsovasilis
[k8s]: transition integration tests to adapter pattern (#6277) (#6357)
78ecd6d 
* feat: transition kubernetes integration tests to adapter pattern

* fix: count nodes without NoSchedule taints

(cherry picked from commit 67c744f)

Co-authored-by: Panos Koutsovasilis <panos.koutsovasilis@elastic.co>
pkoutsovasilis added a commit that referenced this pull request Dec 18, 2024
@mergify @pkoutsovasilis
[k8s]: transition integration tests to adapter pattern (#6277) (#6358)
a65b482 
* feat: transition kubernetes integration tests to adapter pattern

* fix: count nodes without NoSchedule taints

(cherry picked from commit 67c744f)

Co-authored-by: Panos Koutsovasilis <panos.koutsovasilis@elastic.co>
pkoutsovasilis added a commit that referenced this pull request Dec 19, 2024
@mergify @pkoutsovasilis
[k8s]: transition integration tests to adapter pattern (#6277) (#6359)
8640228 
* feat: transition kubernetes integration tests to adapter pattern

* fix: count nodes without NoSchedule taints

(cherry picked from commit 67c744f)

Co-authored-by: Panos Koutsovasilis <panos.koutsovasilis@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@swiatekm swiatekm swiatekm approved these changes

@pchila pchila pchila approved these changes

@blakerouse blakerouse Awaiting requested review from blakerouse blakerouse is a code owner automatically assigned from elastic/elastic-agent-control-plane

Assignees

@pkoutsovasilis pkoutsovasilis

Labels
backport-8.x Automated backport to the 8.x branch with mergify backport-8.16 Automated backport with mergify backport-8.17 Automated backport with mergify skip-changelog Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team Testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pkoutsovasilis @elasticmachine @swiatekm @pchila