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

validate kubernetes templates in .CI #417

Merged
merged 8 commits into from
May 10, 2022
Merged

Conversation

narph
Copy link
Contributor

@narph narph commented May 10, 2022

What does this PR do?

Adds validation of kube templates in CI

Why is it important?

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 change 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.next.asciidoc or CHANGELOG-developer.next.asciidoc.

@narph narph requested a review from a team as a code owner May 10, 2022 10:11
@narph narph requested review from ph and michel-laterman and removed request for a team May 10, 2022 10:11
@mergify mergify bot assigned narph May 10, 2022
@mergify
Copy link
Contributor

mergify bot commented May 10, 2022

This pull request does not have a backport label. Could you fix it @narph? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 8./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

@elasticmachine
Copy link
Contributor

elasticmachine commented May 10, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-05-10T12:57:01.117+0000

  • Duration: 19 min 12 sec

Test stats 🧪

Test Results
Failed 0
Passed 3849
Skipped 21
Total 3870

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages.

  • run integration tests : Run the Elastic Agent Integration tests.

  • run end-to-end tests : Generate the packages and run the E2E Tests.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@elasticmachine
Copy link
Contributor

elasticmachine commented May 10, 2022

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 95.652% (66/69) 👍
Files 69.524% (146/210) 👍
Classes 69.194% (292/422) 👍
Methods 52.493% (800/1524) 👍
Lines 38.735% (8598/22197) 👎 -0.018
Conditionals 100.0% (0/0) 💚

@narph narph requested a review from v1v May 10, 2022 10:37
@narph narph added Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team backport-v8.2.0 labels May 10, 2022
@mergify mergify bot removed the backport-skip label May 10, 2022
@narph narph requested a review from ChrsMark May 10, 2022 10:38
@narph
Copy link
Contributor Author

narph commented May 10, 2022

/test

.ci/Jenkinsfile Outdated
@@ -61,6 +61,7 @@ pipeline {
dir("${BASE_DIR}"){
setEnvVar('BEAT_VERSION', sh(label: 'Get beat version', script: 'make get-version', returnStdout: true)?.trim())
log(level: 'INFO', text: "env.BEAT_VERSION=${env.BEAT_VERSION}")
cmd(label: 'check', script: 'make -C deploy/kubernetes all')
Copy link
Member

Choose a reason for hiding this comment

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

I'd say to move this to the k8s specific stage, see L153, the k8s stage runs only if there are changes matching the regex (^deploy/kubernetes/.*|^version/docs/version.asciidoc) (see L54) on a PR basis or by default on branches

Copy link
Member

Choose a reason for hiding this comment

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

OTOH, what's the difference between make -C deploy/kubernetes all and make -C deploy/kubernetes test, since test is already in place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

make -C deploy/kubernetes all generates the kube*.yml files after which a MAKE check-no-changes should be ran to make sure the templates are up to date. This step should take place in the Check area

Copy link
Member

Choose a reason for hiding this comment

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

What do you think if:

  1. make -C deploy/kubernetes all could be renamed to a different make goal, so the name of the goal reflects what it does.
  2. change make check-ci to run the above make goal.

elastic-agent/Makefile

Lines 44 to 48 in 63b682f

.PHONY: check-ci
check-ci:
@mage update
@$(MAKE) notice
@$(MAKE) check-no-changes

With the above, there is no need to change the pipeline, there is only one make goal to run the checks either locally or in the CI, and additionally the make goal all could reflect what it does

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no issues with that option as well, made the necessary changes

@narph narph requested a review from v1v May 10, 2022 12:27
@mergify
Copy link
Contributor

mergify bot commented May 10, 2022

This pull request does not have a backport label. Could you fix it @narph? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 8./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@narph narph requested a review from v1v May 10, 2022 12:56
@v1v
Copy link
Member

v1v commented May 10, 2022

What's the reason the changes in the CI and Makefile are not backported to 8.2?

@narph
Copy link
Contributor Author

narph commented May 10, 2022

What's the reason the changes in the CI and Makefile are not backported to 8.2?

we could, we just need to update the version to 8.2

@mergify mergify bot removed the backport-skip label May 10, 2022
@narph narph merged commit 23238de into elastic:main May 10, 2022
@narph narph deleted the kube-ci-valid branch May 10, 2022 13:34
mergify bot pushed a commit that referenced this pull request May 10, 2022
* update version

* test valiadtion

* invalid

* invalid

* valid

* move func

* move func

(cherry picked from commit 23238de)

# Conflicts:
#	version/docs/version.asciidoc
narph added a commit that referenced this pull request May 10, 2022
* validate kubernetes templates in .CI (#417)

* update version

* test valiadtion

* invalid

* invalid

* valid

* move func

* move func

(cherry picked from commit 23238de)

# Conflicts:
#	version/docs/version.asciidoc

* update version

Co-authored-by: Mariana Dima <[email protected]>
v1v added a commit to v1v/elastic-agent that referenced this pull request May 18, 2022
…use-orka

* 'main' of github.com:elastic/elastic-agent: (23 commits)
  [Automation] Update go release version to 1.17.10 (elastic#432)
  [Automation] Update elastic stack version to 8.3.0-4149272f for testing (elastic#435)
  [Automation] Update elastic stack version to 8.3.0-19aba912 for testing (elastic#430)
  Add extra k8s resources in clusterRole (elastic#424)
  [Automation] Update elastic stack version to 8.3.0-8ee1196f for testing (elastic#422)
  [Automation] Update elastic stack version to 8.3.0-53513548 for testing (elastic#421)
  Add tags option during enroll/install (elastic#336)
  validate kubernetes templates in .CI (elastic#417)
  add missing kube-api resources from managed agent manifest (elastic#381)
  Create snyk-scan.yml (elastic#397)
  [Automation] Update elastic stack version to 8.3.0-d380914f for testing (elastic#414)
  [Automation] Update elastic stack version to 8.3.0-5c1ff35f for testing (elastic#413)
  [Automation] Update elastic stack version to 8.3.0-6ba9f710 for testing (elastic#410)
  [Automation] Update elastic stack version to 8.3.0-a1c5cfff for testing (elastic#406)
  [Automation] Update elastic stack version to 8.3.0-7f585873 for testing (elastic#401)
  [Automation] Update elastic stack version to 8.3.0-0b6ea9f2 for testing (elastic#399)
  ci: enable coverage (elastic#377)
  Remove last dependencies on beats repo (elastic#387)
  Remove dependency on libbeat (elastic#344)
  [Automation] Update elastic stack version to 8.3.0-cb2ce38c for testing (elastic#383)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v8.2.0 Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants