cloudfoundry / diego-release

BOSH Release for Diego
Apache License 2.0
201 stars 212 forks source link

Add cnb_app_lifecycle #929

Closed c0d1ngm0nk3y closed 2 months ago

c0d1ngm0nk3y commented 4 months ago

Summary

This adds another app lifecycle called cnbapplifecycle to the diego-release to allow apps to build and start with cloud native buildpacks.

Backward Compatibility

Breaking Change? No

linux-foundation-easycla[bot] commented 4 months ago

CLA Signed

The committers listed above are authorized under a signed CLA.

PlamenDoychev commented 4 months ago

Looks good to me. This PR is basically adding a new lifecycle in diego release. The communication around the new 'cnb' lifecycle is documented in the RFC 28.

winkingturtle-vmw commented 4 months ago

Thanks for doing this work. I am a bit newbie when it comes to CNB, so I ask you to please help me get educated along the way. I looked through the linked issues and was not sure if there is an overall acceptance-criteria for this feature. Something in the following format would be good to start with

Scenario: describe scenario Given I have some sort of configuration When I do X And do Y Then I see the desired behavior

ameowlia commented 4 months ago

👋 Hi @c0d1ngm0nk3y,

There is a little bit of awkwardness about this integration that I want to call out. One group is developing this repo, but a different group of people maintain diego release. ❓ Are you going to handle bumping go/dependancies/etc? ❓ Or should we handle that in the diego-release pipeline like we do with other submodules? ❓ How do you think we should handle the maintenance long term between these two groups?

loewenstein-sap commented 4 months ago

There is a little bit of awkwardness about this integration that I want to call out. One group is developing this repo, but a different group of people maintain diego release. ❓ Are you going to handle bumping go/dependancies/etc? ❓ Or should we handle that in the diego-release pipeline like we do with other submodules? ❓ How do you think we should handle the maintenance long term between these two groups?

Hi @ameowlia, we planned to bump the dependencies in the cnbapplifecycle repo directly, likely with dependabot or renovate. Unless of course we should not because the general practice in the WG is to manage Go dependencies in the Bosh release. We might want to discuss challenging the status quo then though ;) because we like to be able to build the Go binary without the context of the Diego release.

Not sure what the long-term maintenance implications will be. We need to add CI to the repo (GitHub actions, if that's acceptable) and when we release, it of course needs to be bumped in the Diego release - we'll likely open PRs for this automatically. Also, we'll happily welcome contributors and/or maintainers to the new lifecycle repo.

loewenstein-sap commented 4 months ago

Thanks for doing this work. I am a bit newbie when it comes to CNB, so I ask you to please help me get educated along the way. I looked through the linked issues and was not sure if there is an overall acceptance-criteria for this feature. Something in the following format would be good to start with

I'd probably go with Scenario: Deploying applications to Cloud Foundry and using Cloud Native Buildpacks for the staging process Given I have a Java application When I do cf push --lifecycle cnb -b paketobuildpacks/java Then I will see the output of Paketo Buildpacks and the CNB lifecycle in my application staging log and will end up with my application running on CF.

Does that help @winkingturtle-vmw?

More context can be found in buildpacks.io and paketo.io.

ameowlia commented 4 months ago

Here are the four things that are top of mind for me and how they happen right now for other diego-release submodules.

1️⃣ Where will test be run All tests for all current submodules are run in the diego-release pipeline in concourse. We have it set up so that all tests are run with the same go version that is set at the bosh release level.

2️⃣ Bumping submodules in diego release All submodules are bumped daily in the diego-relase pipeline in concourse. This will happen automatically (I think) if this PR is merged.

3️⃣ Go Updates Go is bumped at the diego release level. This is done in the diego release pipeline in concourse.

4️⃣ Dependency Bumps Each repo uses dependabot and they are either automatically or manually merged.

I think we should work to keep to this pattern as much as possible. We have our working group meeting in 5 minutes :) I'm sure we'll talk more about it there.

geofffranks commented 4 months ago

If cnb-app-lifecycle doesn't need to import any of the go package from diego-release/src/code.cloudfoundry.org submodules, there's possibly another option that would allow cnb-app-lifecycle to be much more independent from diego-release:

We do this pattern already in at least nats-release. Would this be any easier for all of you and seem like a reasonable alternative?

loewenstein commented 4 months ago

Here are all the PRs for the implementation of the RFC for reference:

c0d1ngm0nk3y commented 3 months ago

If cnb-app-lifecycle doesn't need to import any of the go package from diego-release/src/code.cloudfoundry.org submodules, there's possibly another option that would allow cnb-app-lifecycle to be much more independent from diego-release:

Sounds good. We will try this.

  • Instead of adding cnb-app-lifecycle as a submodule, we add the it as an import to the diego-release/src/code.cloudfoundry.org/tools package (or something similar)

Understood.

  • the diego-release package definition would then pull in source from src/code.cloudfoundry.org/vendor/...cnb-app-lifecycle

But how to maintain the list of vendored folders in the spec file, like here.

  • diego-release CI will bump the cnb-app-lifecycle go package when it detects newly released versions of cnb-app-lifecycle

But how to synch on the dependencies? There could be conflicts, right?

We do this pattern already in at least nats-release. Would this be any easier for all of you and seem like a reasonable alternative?

We had a look, could you please check above question.

winkingturtle-vmw commented 3 months ago

But how to maintain the list of vendored folders in the spec file, like here.

This would be accomplished by running the linter. We update those depdendencies by running this daily job.

c0d1ngm0nk3y commented 3 months ago

@winkingturtle-vmw, @geofffranks , @ameowlia We tried to use ad cnbapplifecycle as a go dependency here. Would this be an option?

Open question: Will the automation take care of the vendoring and the spec file for copying the dependencies or do we need to do something?

winkingturtle-vmw commented 3 months ago

@c0d1ngm0nk3y If you add

_  "code.cloudfoundry.org/cnbapplifecycle/cmd/builder"
_  "code.cloudfoundry.org/cnbapplifecycle/cmd/launcher"

to modules.go and then from src/code.cloudfoundry.org directory you run:

go mod tidy
go mod vendor

It will bring all of the required dependencies. You then have to add cnbapplifecyle to the list of packages to use gosub replacement. After that when you run the linter (linked above), it should populate the spec file with all of the required files to be used at bosh compile time. I hope that helps.

modulo11 commented 3 months ago

We removed the Windows build and adjusted the PR according to the instructions. @winkingturtle-vmw could you please verify that we did not miss anything?

modulo11 commented 3 months ago

@winkingturtle-vmw we added the license and cut a new release.

winkingturtle-vmw commented 3 months ago

@modulo11 this looks good now. We now need someone to deploy this release and verify it's functionality. We have a story for this PR.

loewenstein commented 3 months ago

@winkingturtle-vmw what is the status of this - and what's the plan? I.e. what kind of test do you plan to do - just whether this cleanly installs with Bosh I guess? Not sure if it helps, but we do have it running in a bbl up based installation and did not run into any issues.

winkingturtle-vmw commented 3 months ago

@loewenstein There is a story in our backlog to deploy this PR and verify it's functionality. We are also going to turn on CATs tests in our pipeline for this feature. Additionally, I don't see any information around what CNBs are supported in the first iteration of this feature and plans around supporting other CNBs in the future. It would be helpful for the community to know the roadmap for this feature (maybe as a markdown in cnbapplifecycle repo or a series of issues) since it's getting added for every deploy of CF. We don't have anyone assigned to the story yet, but it's in our backlog and we will get to it.

loewenstein-sap commented 3 months ago

Thanks @winkingturtle-vmw for the response.

I'd argue that this PRs functionality is pretty limited as it only adds another lifecycle binary to the file server. The end to end functionality, I.e. the support for Cloud Native Buildpacks, is not available until the cf-deployment PR is merged and it is turned off by default via Cloud Controller feature gate. Wondering if the Diego-release PR is the right place for e2e checks.

Regarding the community questions: all Cloud Native Buildpacks that can tolerate cflinuxfs4 as rootfs are going to be supported. There is a follow up rfc already merged to support cloud native Buildpacks as system Buildpacks and there should likely be a follow up to decide if there should be batteries included and which ones - I'd assume the answer will be "Yes" and "Paketo" but that is still open in terms of rfc and work distribution - probably something for the ARI WG and Buildpacks maintainers.

Once the functionality is actually available, i.e. all remaining PRs merged, it's probably time to add cloud native buildpack support to CF Docs.

TL;DR the near term roadmap for Buildpacks is documented in the RFCs, user facing documentation blocked on the open PRs and I would hope to get this PR merged soon and hopefully disentangled from an e2e test run that includes ~10 PRs with more than half of them actually already merged anyway.

loewenstein-sap commented 2 months ago

@winkingturtle-vmw @ameowlia Any chance to get this in? It's one of the last pieces of the puzzle - only CF CLI and cf-deployment left and there's also RFC31 System CNBs already merged and waiting for implementation.

mariash commented 2 months ago

@loewenstein-sap to update on status. I tried to deploy the code, but have been getting go mod compilation errors. Trying to understand what is happening..

loewenstein-sap commented 2 months ago

@loewenstein-sap to update on status. I tried to deploy the code, but have been getting go mod compilation errors. Trying to understand what is happening..

Hi @mariash, could you please provide details about what kind of error you are seeing.

mariash commented 2 months ago

@loewenstein-sap this is the error I am getting:

Compiling packages: cnb_app_lifecycle/b0a87785b34b6d4ba94fde319428ba3409e9618695a77c1a16c8b3f0acb471e7 (00:01:32)
                   L Error: Action Failed get_task: Task e6b81a0e-7df8-4b01-50d5-ee6e23510f78 result: Compiling package cnb_app_lifecycle: Running packaging script: Running packaging script: Command exited with 1; Stdout: , Truncated stderr:   github.com/creack/pty@v1.1.21: is explicitly required in go.mod, but not marked as explicit in
vendor/modules.txt
        github.com/cyphar/filepath-securejoin@v0.3.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt
        github.com/dimchansky/utfbom@v1.1.1: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt
        github.com/distribution/reference@v0.6.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt
        github.com/docker/cli@v26.1.3+incompatible: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt
        github.com/docker/distribution@v2.8.3+incompatible: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt
        github.com/docker/docker@v27.0.3+incompatible: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt
        github.com/docker/docker-credential-helpers@v0.8.2: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt
        github.com/docker/go-connections@v0.5.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt
        github.com/docker/go-metrics@v0.0.1: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt
        github.com/docker/go-units@v0.5.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.tx
...
mariash commented 2 months ago

The steps to reproduce:

bosh create-release
bosh upload-release
bosh deploy
mariash commented 2 months ago

@aminjam noticed an issue with the spec file missing modules.txt file, once I added it deploy succeeded

c0d1ngm0nk3y commented 2 months ago

The steps to reproduce:

@mariash Thanks. With that input, I was able to:

mariash commented 2 months ago

@c0d1ngm0nk3y Hi, could you please rebase onto the latest code. Our CI is green now and we paused it so that nothing gets merged until your PR is merged.

loewenstein commented 2 months ago

@mariash Is there a diego-release release planned already that will contain this? It's going to be needed to merge https://github.com/cloudfoundry/cf-deployment/pull/1178.