Skip to content

Conversation

@axelaris
Copy link
Contributor

Hi guys,
this is scripts to generate manifests for Smoke and Acceptance tests as long as main deployment manifest.

@cfdreddbot
Copy link

Hey axelaris!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you've already signed the CLA.

1 similar comment
@cfdreddbot
Copy link

Hey axelaris!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you've already signed the CLA.

@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this. You can view the current status of your issue at: https://www.pivotaltracker.com/story/show/109012232.

@emalm
Copy link
Contributor

emalm commented Dec 1, 2015

Thanks, @axelaris, it'd be great to have separate scripts to generate the acceptance-test and smoke-test deployment manifests. I think this would conflict heavily both with the changes we intend to pull in with #109 and with the flag-based arguments we discussed on #103, though, so it would probably work better for at least the #109 changes to go in first before we develop these particular scripts.

Thanks,
Eric

@axelaris
Copy link
Contributor Author

axelaris commented Dec 1, 2015

Ok, waiting for #109 to me merged first.

@axelaris
Copy link
Contributor Author

axelaris commented Dec 7, 2015

Hi @ematpl,
I've fixed my patch to intend #109.
Could you please check it.

@emalm
Copy link
Contributor

emalm commented Dec 8, 2015

Thanks, @axelaris! I think it would be great for these scripts to follow the argument conventions we discussed in #103 that come from https://www.pivotaltracker.com/n/projects/1003146/stories/107891444. Specifically, it would make sense for them to take the following mandatory argument flags, all mandatory:

  • -c: CF manifest file
  • -i: IaaS-settings stub file
  • -p: property-overrides stub file

For consistency with generate-deployment-manifest, the scripts could also take an optional -v argument with a stub that specifies the version of diego-release, with the version in the manifest defaulting to 'latest' if the version isn't specified. That could be done later, though.

The scripts also don't need to take an argument for the template, as #109 introduces a generic template for the DATs at https://github.com/cloudfoundry-incubator/diego-release/blob/develop/manifest-generation/diego-acceptance-tests.yml and for the smoke tests at https://github.com/cloudfoundry-incubator/diego-release/blob/develop/manifest-generation/diego-smoke-tests.yml that we can refer to in the script itself.

The spiff merge sequence should also include https://github.com/cloudfoundry-incubator/diego-release/blob/develop/manifest-generation/misc-templates/bosh.yml as the left-most template argument to allow only the BOSH-recognized top-level keys to filter through.

Does that makes sense? I'm happy to discuss it more, if you're still up for contributing the scripts.

Thanks again,
Eric

@axelaris
Copy link
Contributor Author

axelaris commented Dec 9, 2015

Hi @ematpl,
I'm done.
Could you please check?

@emalm
Copy link
Contributor

emalm commented Dec 10, 2015

Thanks for the changes, @axelaris!

I'm pretty sure we want the ${manifest_generation}/misc-templates/bosh.yml template to be the first argument to the final spiff merge in each script, rather than an intermediate argument in the first spiff merge. That bosh.yml template will filter out the non-BOSH-y top-level keys such as config_from_cf that the diego.yml template includes on https://github.com/cloudfoundry-incubator/diego-release/blob/develop/manifest-generation/diego-acceptance-tests.yml#L48-L51 only so it can receive the inputs from later stub arguments.

Also, it would be great for the script to tell the user about all the arguments that are missing or invalid, rather than just one of them at a time. So then if I accidentally invoke the script without the -c flag at all and with a directory instead of a regular file for the -p flag, the error message from the script tells me about both errors before printing the usage, rather than just, say, the missing -c argument. Having that more complete set of validations can really help reduce the iteration time when using the script, especially when it's invoked in the middle of a complicated, expensive testing pipeline. But that's a nice-to-have feature. :)

Thanks again,
Eric

@axelaris
Copy link
Contributor Author

@ematpl, done, with nice-to-have features!

@emalm
Copy link
Contributor

emalm commented Dec 21, 2015

Thanks again, @axelaris! Looks good to me! Prioritized for the Diego team to evaluate this coming week.

Best,
Eric

@jfmyers9 jfmyers9 merged commit 99a0c8d into cloudfoundry:develop Dec 22, 2015
tas-runtime-bot added a commit that referenced this pull request May 23, 2023
Submodule src/code.cloudfoundry.org/credhub-cli 4bcb92fdb..439bdb21c:
  > Bump go modules
Submodule src/garden 234178722..8444ff5a3:
  > Merge pull request #110 from cloudfoundry/d-tracing
Submodule src/grootfs 3eb401079..7488cb1e0:
  > Bump github.com/cloudfoundry/dropsonde from 1.0.0 to 1.1.0
  > Bump github.com/docker/docker
Submodule src/guardian f34a4cc2c..3f995a45f:
  > Vendor latest garden with distributed tracing
tas-runtime-bot added a commit that referenced this pull request Mar 25, 2025
…guardian idmapper

Submodule src/code.cloudfoundry.org/buildpackapplifecycle eb9ce4e2a..36f93002c:
  > Merge pull request #75 from cloudfoundry/vet-Sprintf
Submodule src/code.cloudfoundry.org/cfdot 096ea81..89695eb:
  > Change SetOutput to SetOut (deprecated, fails staticcheck) (#15)
Submodule src/code.cloudfoundry.org/credhub-cli 6d4d60c75..abc61188c:
  > Bump go modules
Submodule src/code.cloudfoundry.org/executor 974aef6..53c8c2e:
  > Fix error with fmt.Sprintf and non-constant formats
  > update key-size for test certs to be ready for go 1.24
  > Add envoy proxy liveness checks (#110)
Submodule src/garden 1d757121f..7b68590a2:
  > Update go.mod dependencies
  > Add anon, file, and swap for cgroups v2
  > Update go.mod dependencies
Submodule src/grootfs f5f2ad919..68c02aeca:
  > Update go.mod dependencies
  > Update go.mod dependencies
Submodule src/guardian 861472c65..711f3a7c0:
  > Update go.mod dependencies
  > In cgroups v2 we need to use different fields for memory
  > Update go.mod dependencies
Submodule src/idmapper 3a0647dea..24cb87edf:
  > Update go.mod dependencies
  > Update go.mod dependencies
tas-runtime-bot added a commit that referenced this pull request Apr 1, 2025
Submodule src/code.cloudfoundry.org/auctioneer 66c25b0..c956679:
  > [Add BBS Advanced Metrics] Adapt auctioneer to the changes in BBS. (#21)
Submodule src/code.cloudfoundry.org/bbs f6b7ab2..4ca7ffa:
  > Add BBS Advanced Metrics (#110)
Submodule src/code.cloudfoundry.org/credhub-cli abc61188c..1c59575f4:
  > Bump go modules
Submodule src/garden 7b68590a2..bc5d6fb7c:
  > Update go.mod dependencies
Submodule src/grootfs 68c02aeca..5efe6f0db:
  > Update go.mod dependencies
Submodule src/guardian 711f3a7c0..c2442f91f:
  > Update go.mod dependencies
kart2bc pushed a commit to kart2bc/diego-release that referenced this pull request Aug 19, 2025
* [BBSAdvancedMetrics] Add Config

* [BBSAdvancedMetrics] 04-02

* implement `RecordMetrics`

* fix dependency with auctioneer

* fix formatting

* regenerate fake-emitter

* fix broken tests

* [BBSAdvancedMetrics] cleanup

* [BBSAdvancedMetrics] fix emit metrics inconsistency

* fix config-data formatting

* remove unused GetAdvancedMetricsConfig

* introduce initRoutes

* [AdvancedMetrics] add route validation when Advanced Metrics are enabled && change call order in incrementRequestCount

* [AdvancedMetrics] add unit tests

* fix handlers.go file
this change was missed during rebase

---------

Co-authored-by: Stanimir Petrov <[email protected]>
kart2bc pushed a commit to kart2bc/diego-release that referenced this pull request Aug 25, 2025
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