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

Migrate to the new chart repo #200

Merged
merged 3 commits into from
Nov 20, 2019
Merged

Migrate to the new chart repo #200

merged 3 commits into from
Nov 20, 2019

Conversation

pivotal-bin-ju
Copy link
Contributor

@pivotal-bin-ju pivotal-bin-ju commented Nov 8, 2019

fixes https://github.com/pivotal/concourse-ops/issues/117

We moved our helm chart out of the helm/charts repo and over to
concourse/concourse-chart

To make releasing and testing easier for us we're going to follow a
branching strategy similar to the concourse-bosh-deployment repo.

For the main concourse pipeline all tests will be done against a dev
branch that stays updated with master and has other commits that are
for unreleased changes currently on the master branch of
concourse/concourse

To reclarify, concourse/concourse-chart has two branches:

  • master reflects the current OSS release of the chart
  • dev reflects master plus other commits related to unreleased
    versions of concourse/concourse

Currently this branching stragegy has no automation:

  • dev not being promoted to master
  • dev not being rebased on master before testing
  • need to add lint checking for the pr against our helm chart

Related PR: concourse/concourse#4735

cc @taylorsilva

@taylorsilva taylorsilva force-pushed the chart-repo-migration branch 3 times, most recently from 5294f42 to 497b96c Compare November 13, 2019 21:54
@taylorsilva taylorsilva marked this pull request as ready for review November 13, 2019 21:57
@taylorsilva taylorsilva requested a review from a team November 13, 2019 21:57
@taylorsilva taylorsilva changed the title [fix concourse-ops#117] migrate to the new chart repo Migrate to the new chart repo Nov 13, 2019
We moved our helm chart out of the `helm/charts` repo and over to
concourse/concourse-chart

To make releasing and testing easier for us we're going to follow a
branching strategy similar to the concourse-bosh-deployment repo.

For the main concourse pipeline all tests will be done against a `dev`
branch that stays updated with master and has other commits that are
for unreleased changes currently on the master branch of
concourse/concourse

To reclarify, concourse/concourse-chart has two branches:
- `master` reflects the current OSS release of the chart
- `dev` reflects `master` plus other commits related to unreleased
versions of `concourse/concourse`

Currently this branching stragegy has no automation:
- `dev` not being promoted to master
- `dev` not being rebased on master before testing
- need to add lint checking for the pr against our helm chart

Co-authored-by: Taylor Silva <tsilva@pivotal.io>
Signed-off-by: Bin Ju <bju@pivotal.io>
Copy link
Member

@cirocosta cirocosta left a comment

Choose a reason for hiding this comment

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

Hey,

Most of it seems pretty good 😁 It seems like there are some newly introduced passed constraints that would fail though.

Thanks!

@@ -335,7 +335,9 @@ jobs:
- get: unit-image
passed: [k8s-smoke]
trigger: true
- get: charts
- get: helm-charts
passed: [k8s-smoke]
Copy link
Member

Choose a reason for hiding this comment

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

Hey,

is this new passed constraint necessary? If I got it right, this one will make setting the pipeline fail, as k8s-smoke does not get helm-charts 🤔

If it is, then we'd need to add it there (have a get), or, make it passed on something that uses it.

thx!

passed: [k8s-smoke]
tags: [k8s-topgun]
trigger: true
- get: helm-charts
passed: [k8s-smoke]
Copy link
Member

Choose a reason for hiding this comment

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

same as for branch.yml - this addition here will make setting the pipeline fail

- get: charts
- get: concourse-chart
passed: [k8s-smoke]
- get: helm-charts
passed: [k8s-smoke]
Copy link
Member

Choose a reason for hiding this comment

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

this passed constraint will be problematic (same as above)

- k8s-topgun is the only job that needs to use other charts
  from the helm/charts repo. k8s-topgun doesn't need to check
  that helm/charts passed on other jobs, since we use
  concourse/concourse-chart in favor of helm/charts for those
  jobs now.

Signed-off-by: Denise Yu <dyu@pivotal.io>
@deniseyu deniseyu dismissed cirocosta’s stale review November 20, 2019 19:41

Addressed in new changes

Copy link
Member

@cirocosta cirocosta left a comment

Choose a reason for hiding this comment

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

we verified that the pipelines can now be set

@deniseyu deniseyu merged commit e1c4441 into master Nov 20, 2019
@cirocosta cirocosta deleted the chart-repo-migration branch November 20, 2019 19:51
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.

3 participants