Skip to content

Feature: CNF Installation (4) Group several manifest into one file#2150

Merged
martin-mat merged 1 commit into
mainfrom
#2125-create-common-manifest
Oct 24, 2024
Merged

Feature: CNF Installation (4) Group several manifest into one file#2150
martin-mat merged 1 commit into
mainfrom
#2125-create-common-manifest

Conversation

@barmull
Copy link
Copy Markdown
Collaborator

@barmull barmull commented Sep 17, 2024

Description

  • Gather all the manifest files from the various CNFs into a single file called common_manifests.yaml

  • Modified temp_template.yaml to reference common_manifests.yaml

  • Removed code responsible for generating CNF templates as it is no longer used

  • Added functionality in cleanup.cr to remove common_manifests.yaml as part of the cleanup process

  • Update generated manifest, add namespace to each resource

  • Depends on PR in helm library: Feature: Add method generate_manifest cnf-testsuite/helm#4

Issues:

Refs: #2125

Re-created from different branch, old PR: #2144

@barmull barmull force-pushed the #2125-create-common-manifest branch 2 times, most recently from 3a123e2 to 0d92ced Compare September 20, 2024 13:05
Comment thread src/tasks/utils/cnf_installation/manifest.cr Outdated
Comment thread src/tasks/utils/cnf_installation/manifest.cr Outdated
Comment thread src/tasks/utils/cnf_installation/manifest.cr Outdated
@barmull barmull force-pushed the #2125-create-common-manifest branch from 0d92ced to 370c35f Compare September 24, 2024 15:03
Copy link
Copy Markdown
Collaborator

@kosstennbl kosstennbl left a comment

Choose a reason for hiding this comment

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

Nice change!
We should also not forget to modify shard files after PR in Helm repo would be merged.

Some comments and possible changes below:

Comment thread src/tasks/cleanup.cr Outdated
Comment thread src/tasks/utils/cnf_installation/manifest.cr Outdated
Comment thread src/tasks/utils/cnf_installation/manifest.cr Outdated
Comment thread src/tasks/utils/cnf_installation/manifest.cr Outdated
Comment thread src/tasks/utils/cnf_installation/manifest.cr Outdated
Comment thread src/tasks/utils/cnf_manager.cr Outdated
@barmull barmull force-pushed the #2125-create-common-manifest branch 2 times, most recently from 8bdf96c to 552616c Compare September 27, 2024 14:34
@kosstennbl kosstennbl self-requested a review September 30, 2024 12:27
Comment thread src/tasks/cleanup.cr Outdated
Comment thread src/tasks/utils/cnf_installation/manifest.cr Outdated
Comment thread src/tasks/utils/cnf_installation/manifest.cr Outdated
Comment thread src/tasks/utils/cnf_installation/manifest.cr Outdated
Comment thread src/tasks/utils/cnf_installation/manifest.cr
Comment thread src/tasks/utils/cnf_installation/manifest.cr Outdated
Comment thread src/tasks/utils/cnf_installation/manifest.cr Outdated
Comment thread src/tasks/utils/cnf_manager.cr Outdated
Comment thread src/tasks/utils/cnf_manager.cr Outdated
Comment thread src/tasks/utils/cnf_manager.cr
@barmull barmull force-pushed the #2125-create-common-manifest branch 3 times, most recently from dc84367 to 9713630 Compare October 3, 2024 14:58
Comment thread src/tasks/utils/cnf_installation/manifest.cr Outdated
Comment thread src/tasks/utils/cnf_manager.cr Outdated
Comment thread src/tasks/utils/cnf_manager.cr
Comment thread src/tasks/utils/cnf_manager.cr Outdated
@barmull barmull force-pushed the #2125-create-common-manifest branch 3 times, most recently from d57ddca to 0a83d77 Compare October 4, 2024 13:34
@kosstennbl
Copy link
Copy Markdown
Collaborator

Tested, found some mistakes:

common_manifest is not deleted on cnf_cleanup (should be)
doesn't use last helm version in shards (is required for this PR).

@kosstennbl kosstennbl marked this pull request as draft October 7, 2024 09:14
@barmull barmull force-pushed the #2125-create-common-manifest branch 3 times, most recently from ba61349 to cebe752 Compare October 7, 2024 16:53
Comment thread src/tasks/utils/cnf_manager.cr Outdated
@kosstennbl
Copy link
Copy Markdown
Collaborator

Requires #2149 and #2162 to be merged to pass all tests and be ready for merging

@barmull barmull force-pushed the #2125-create-common-manifest branch from cebe752 to f4021c8 Compare October 8, 2024 12:41
@martin-mat martin-mat self-requested a review October 8, 2024 14:50
@barmull
Copy link
Copy Markdown
Collaborator Author

barmull commented Oct 10, 2024

Also need to add namespaces into workload resources by updating generated_manifest.

@barmull barmull force-pushed the #2125-create-common-manifest branch from f4021c8 to bd35849 Compare October 11, 2024 13:56
@barmull barmull force-pushed the #2125-create-common-manifest branch 6 times, most recently from 14a6bff to 7ee8aa8 Compare October 17, 2024 13:15
Copy link
Copy Markdown
Collaborator

@kosstennbl kosstennbl left a comment

Choose a reason for hiding this comment

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

Some changes, mostly cosmetic

Comment thread src/tasks/utils/cnf_installation/manifest.cr Outdated
Comment thread src/tasks/utils/cnf_installation/manifest.cr Outdated
Comment thread src/tasks/utils/cnf_manager.cr Outdated
Comment thread src/tasks/utils/cnf_manager.cr Outdated
Comment thread src/tasks/utils/k8s_tshark.cr Outdated
Comment thread src/tasks/utils/timeouts.cr Outdated
Comment thread src/tasks/workload/5g_validator.cr Outdated
Comment thread src/tasks/workload/5g_validator.cr Outdated
Comment thread src/tasks/workload/microservice.cr Outdated
Comment thread src/tasks/workload/microservice.cr Outdated
@barmull barmull force-pushed the #2125-create-common-manifest branch from 7ee8aa8 to 642dc1d Compare October 21, 2024 09:34
Copy link
Copy Markdown
Collaborator

@kosstennbl kosstennbl left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread src/tasks/utils/cnf_installation/manifest.cr Outdated
Comment thread src/tasks/utils/cnf_installation/manifest.cr Outdated
@martin-mat
Copy link
Copy Markdown
Collaborator

remove from "Draft"?

@kosstennbl kosstennbl marked this pull request as ready for review October 22, 2024 07:36
@barmull barmull force-pushed the #2125-create-common-manifest branch 2 times, most recently from 212ddc2 to b432047 Compare October 22, 2024 14:29
- group manifests from several CNFs into one
  common_manifests.yaml file
- add functionality in cleanup.cr to remove this file
- depends on PR in helm library:
  cnf-testsuite/helm#4
- common_manifest functionality: used common_manifest
  instead of templates
- remove methods related to templates
- update generated manifest: add namespace to each
  resource

Signed-off-by: barmull <barbora.muller@tietoevry.com>
@barmull barmull force-pushed the #2125-create-common-manifest branch from b432047 to 24744bc Compare October 22, 2024 14:58
@martin-mat martin-mat self-requested a review October 22, 2024 15:10
Copy link
Copy Markdown
Collaborator

@martin-mat martin-mat left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Copy Markdown
Collaborator

@svteb svteb left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Copy Markdown
Collaborator

@collivier collivier left a comment

Choose a reason for hiding this comment

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

lgtm

@martin-mat martin-mat merged commit 5aa3e32 into main Oct 24, 2024
@kosstennbl kosstennbl deleted the #2125-create-common-manifest branch October 29, 2024 11:34
collivier pushed a commit to collivier/testsuite that referenced this pull request Nov 13, 2024
- group manifests from several CNFs into one
  common_manifests.yaml file
- add functionality in cleanup.cr to remove this file
- depends on PR in helm library:
  cnf-testsuite/helm#4
- common_manifest functionality: used common_manifest
  instead of templates
- remove methods related to templates
- update generated manifest: add namespace to each
  resource

Signed-off-by: barmull <barbora.muller@tietoevry.com>
LuciaSirova pushed a commit that referenced this pull request Mar 27, 2025
- group manifests from several CNFs into one
  common_manifests.yaml file
- add functionality in cleanup.cr to remove this file
- depends on PR in helm library:
  cnf-testsuite/helm#4
- common_manifest functionality: used common_manifest
  instead of templates
- remove methods related to templates
- update generated manifest: add namespace to each
  resource

Signed-off-by: barmull <barbora.muller@tietoevry.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants