Skip to content

Conversation

@datianshi
Copy link
Contributor

This is just change the base spiff template diego.yml

So user can provide stub to override/add isolation segments tag

@cfdreddbot
Copy link

Hey datianshi!

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

@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/140929693

The labels on this github issue will be updated when the story is started.

@emalm
Copy link
Contributor

emalm commented Mar 6, 2017

Hi, @datianshi, could you elaborate on how you plan to use a Diego deployment with placement tags configured for isolation segments? We haven't made placement tags configurable on the manifest-generation templates because there's only one set of cell jobs in the manifest, so if that set of jobs received a non-empty placement-tag set then there would be no untagged set of cells to accept CF work in the "default" isolation segment.

Thanks,
Eric, CF Diego PM

@datianshi
Copy link
Contributor Author

@ematpl By default it is still empty... but it opens the possibility to override one AZ cell to have the placement tags without touching the template files. To me it at least give end user both flexibility and education purpose on reading the templates....

bdshroyer pushed a commit that referenced this pull request Mar 27, 2017
[finishes #142522481]

Submodule src/github.com/onsi/ginkgo c3a655f..67b9df7:
  > Remove the spec_iterator.test binary (#336)
  > Shared queue implementation for parallel tests
  > Revert "Don't colorize output by default if not writing to a TTY (#328)" (#331)
  > Don't colorize output by default if not writing to a TTY (#328)
  > Use SVG badge for build status (#330)
  > Add the ability to use ./... to recursively test directories (#319)
  > Include captured output from failed tests into JUnit (#318)
  > Aggregate flaked specs (#316)
  > Add colours for Windows in suite-runner & watch (#312)
  > Fix tests for single node machine (#311)
  > Add ability to specify a custom bootstrap file (#302)
  > Revert "remove -i in invocations of go test.  fixes #305"
  > Update .travis.yml
  > fix imports for generate command (#279)
  > Merge branch 'koron-windows-colorise'
  > remove -i in invocations of go test.  fixes #305
  > remove unnecessary variable
  > backfill GinkgoRandomSeed test
  > Expose the random seed via GinkgoRandomSeed() (#293)
  > Include flake count in test summary (#291)
  > #287 Ensure Logf/Skipf insert newline characters (#288)
  > Add package path prefix to compilation output path only if missing (#284)
  > Redo flags again, add a bunch of pass-throughs. (#282)
  > Spelling fix (#283)
  > Covermode flag (and reworked pass-through flags passing) (#281)
  > Make JUnit reporter include failure location in message. (#262)
  > remove 1.4 from travis.yml
  > Add gcflags option (#276)
  > Revert "Use the go1.5 build tag to handle vendor exceptions" (#274)
  > Merge pull request #272 from fsouza/fix-vendor
  > Add flaky test mitigation (#261)
  > Allow units and precision in benchmark (#266)
  > Add Solaris support (#264)
  > Merge pull request #259 from kwadrat/master
  > Merge branch 'apvail-spell-fix'
  > Fix go16 vendor
  > Merge pull request #250 from james-lawrence/master
  > Merge pull request #228 from jayunit100/RegexFileNameFiltering
  > Fix test flakiness
Submodule src/github.com/onsi/gomega c463cd2..334b8f4:
  > Merge pull request #206 from xoebus/patch-1
  > Merge pull request #205 from onsi/revert-201-json_formatting
  > Merge pull request #201 from madamkiwi/json_formatting
  > Merge pull request #199 from kevgo/patch-1
@emalm
Copy link
Contributor

emalm commented Mar 27, 2017

Thanks for the context, @datianshi. Although it might be convenient for experimentation, I'm hesitant to recommend conflating AZs with isolation segments, and I'd rather have the set of placement tags apply to all 3 AZ-specific cell jobs. One deployment strategy that could work would be to use the manifest-generation scripts to produce a cell-only manifest, all the cells of which have the set of placement tags for the desired isolation segment. That would likely require a different subnet per AZ for those cells (or would require configuring mutually exclusive IP ranges within the same IaaS subnets), in order to prevent BOSH from assigning conflicting IP addresses.

In any case, I think we would accept a PR that took a new optional value in the property-overrides stub (say, property_overrides.rep.placement_tags) and applied it to all 3 cell jobs in the manifest, defaulting to a null value.

Thanks again,
Eric

@emalm
Copy link
Contributor

emalm commented Apr 25, 2017

Hey @datianshi, any interest in making the proposed change to the PR?

Thanks,
Eric

@datianshi datianshi force-pushed the override_isolation branch from c29821b to 11d1f43 Compare April 25, 2017 20:31
@datianshi
Copy link
Contributor Author

@ematpl It makes sense and made the change. Sorry missed that one month ago message

-Shaozhen

@datianshi datianshi force-pushed the override_isolation branch from 11d1f43 to 11b94e5 Compare April 25, 2017 20:40
@datianshi
Copy link
Contributor Author

Just squashed the multiple commits

@emalm
Copy link
Contributor

emalm commented Apr 25, 2017

Thanks, @datianshi! Prioritized for the Diego team to review.

Best,
Eric

@jfmyers9 jfmyers9 merged commit 11b94e5 into cloudfoundry:develop May 2, 2017
@datianshi datianshi deleted the override_isolation branch May 8, 2017 14:48
jvshahid added a commit that referenced this pull request Jun 5, 2018
Submodule src/code.cloudfoundry.org/executor 23ff7f3..08a81a5:
  > use the new gomega Receive matcher to cleanup the tests
Submodule src/github.com/onsi/ginkgo 67b9df7f5..c73579c58:
  > Increase eventually timeout to 30s
  > Clarify asynchronous test behaviour
  > Travis badge should only show master
  > v1.5.0
  > Bring the changelog up to date (#470)
  > Merge pull request #466 from onsi/issue-464
  > Merge pull request #468 from onsi/update-changelog
  > Merge pull request #459 from onsi/unfocus-exclude-vendor
  > Merge pull request #456 from idoru/ignore-suite-with-ignored-go-files
  > Merge pull request #455 from onsi/flaky-measurements
  > Merge pull request #423 from alamages/coverage-tests-fix
  > Add go 1.10 to travis
  > Merge pull request #448 from melnikk/master
  > Merge pull request #451 from onsi/go-1.10-coverage-test
  > Merge pull request #446 from onsi/do-not-create-outputdir
  > Add an extra new line after reporting spec run completion
  > Merge pull request #441 from blgm/releasing
  > Merge pull request #443 from onsi/add-go-1.10
  > Merge pull request #438 from onsi/issue-436
  > Merge pull request #434 from mezis/blur-specify
  > Merge pull request #430 from onsi/issue-426
  > Add tests for When/Specify
  > Update comments and README.md for `When` DSL
  > Merge pull request #386 from akshaymankar/when
  > Merge pull request #383 from eloycoto/master
  > fix the tests on go versions <1.9
  > add a test case for #415
  > Fix Ginkgo stack trace on failure for Specify
  > Merge pull request #409 from lflux/readme-update
  > Merge pull request #400 from alex-slynko/patch-1
  > Merge pull request #403 from alex-slynko/fix_govet
  > Merge pull request #404 from lflux/travis-go-1.9
  > Merge pull request #399 from alex-slynko/fix_imports
  > Merge pull request #401 from alex-slynko/patch-2
  > Merge pull request #398 from martinxsliu/generated-import-ordering
  > Merge pull request #390 from DennisDenuto/master
  > Re-add noisySkippings flag
  > Replace GOPATH in Environment
  > Allow coverage to be displayed for focused specs (#367)
  > ensure customer reporters are run before default reporter
  > Handle -outputdir flag (#364)
  > Handle -coverprofile flag (#355)
  > v1.4.0
  > v1.4.0
  > fix CONTRIBUTING.md
  > Add a CONTRIBUTING.md file
  > add -timeout flag.  fixes #248
  > add 'requireSuite' flag to cause failure if there are test files but no suite (#359)
  > update changelog
  > emit compilation output
  > link to vscode extension
  > add ginkgo watch -watchRegExp fixes #356
  > remove fixCompilationOutput  (fixes #357)
  > Remove mailing list
  > Add Rick and Morty quote to untilItFails messages (#354)
  > Revert "Fix wrong repo URL (#351)" (#352)
  > Fix wrong repo URL (#351)
  > Lack of test suites no longer breaks builds (#347)
  > Update CHANGELOG.md
  > Warn when a suite has test files but no tests to run (#345)
  > fix flaky test by, you know, not relying on obscure go internals
  > update .travis.yml to get latest patch releases
  > fix failing go 1.5 tests
  > ensure tests pass on Go 1.8.1
  > v1.3.1: Actually change the CLI version
  > print location of test suite that failed to report back (#339)
  > v1.3.0
Submodule src/github.com/onsi/gomega 777f4d387..41673fd8f:
  > allow 'Receive' matcher to be used with concrete types (#286)
  > Fix data race in ghttp server (#283)
  > Travis badge should only show master
  > v1.4.0
  > Update the CHANGELOG.md with all changes since last release
  > Improve message from AssignableToTypeOf when expected value is nil (#281)
  > Merge pull request #278 from benmoss/master
  > Merge pull request #273 from vayan/master
  > change Ω to Expect in most places (#268)
  > Merge pull request #272 from trayo/master
  > Merge pull request #271 from onsi/add-go-1.10
  > Merge pull request #270 from Patagonicus/master
  > Remove tests that tried to verify signals as async
  > Merge pull request #262 from onsi/issue-173
  > Merge pull request #259 from onsi/issue-258
  > v1.3.0
  > Add more robust support for XUnit style tests
  > Merge pull request #256 from onsi/allow-multiple-go-routines
  > Merge pull request #253 from challiwill/master
  > Merge pull request #249 from alex-slynko/fix_govet
  > Merge pull request #247 from alex-slynko/fix_imports
  > Merge pull request #243 from williammartin/find-first-mismatch-oob
  > Merge pull request #241 from ivy/fix-typos
  > Merge pull request #218 from alex-slynko/patch-1
  > Merge pull request #239 from williammartin/parallel-testing
  > Merge pull request #235 from onsi/fix-227
  > MatchXML should ignore the order of xml node attributes
  > Merge pull request #233 from kaedys/bugfix/error-matcher-fix
  > Uses equality matcher for matching error strings

Signed-off-by: Sunjay Bhatia <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants