[RFC0028] Support for Cloud Native Buildpacks lifecycle #2871

pbusko · 2024-05-02T13:05:36Z

Where this PR should be backported?

main - all changes should by default start here
v8
v7

Description of the Change

This change introduces changes as described by RFC 0028 Cloud Native Buildpacks Lifecycle.

cf push accepts --lifecycle [buildpacks|docker|cnb] flag
cf push supports lifecycle option in the manifest
cf create-app --app-type extended with cnb option

Why Is This PR Valuable?

What benefits will be realized by the code change? What users would want this change? What user need is this change addressing?

Applicable Issues

List any applicable GitHub Issues here

How Urgent Is The Change?

Depends on cloudfoundry/cloud_controller_ng#3778 to be merged first

Other Relevant Parties

Who else is affected by the change?

loewenstein-sap · 2024-07-23T06:48:20Z

@cloudfoundry/toc Do you know who could be asked to review this one?

beyhan · 2024-07-23T06:58:42Z

Hi @loewenstein-sap,

I asked in the CLI slack channel.

a-b

Implementation LGTM.
Is it possible to add integration tests?

pbusko · 2024-08-05T08:44:52Z

@a-b would it be sufficient to simply enable CNB tests which are part of CATS? cloudfoundry/cf-acceptance-tests#1136 with "include_cnb": true?

Otherwise we would have to basically create test duplicates and test same things multiple times with no additional benefits.

a-b

LGTM

modulo11 · 2024-08-15T08:34:57Z

@a-b could you assist us fixing the failing checks? The CVE check fails to download the Grype DB. Regarding the failed integration tests we are wondering if the targeted CF already supports Cloud Native Buildpacks and if the required feature-flag (diego_cnb) is activated.

a-b · 2024-08-16T16:33:52Z

@a-b could you assist us fixing the failing checks? The CVE check fails to download the Grype DB. Regarding the failed integration tests we are wondering if the targeted CF already supports Cloud Native Buildpacks and if the required feature-flag (diego_cnb) is activated.

At the moment, the integration test is being run against cf-deployment_version: v40.15.0. We do not support modifications to feature flags, but I will discuss with the team to explore potential solutions.

One possible approach to support feature flags would be to introduce a file in the repository with a BOSH opsfile that could be applied prior to running tests. Please, let us know what you think about this approach.

modulo11 · 2024-08-19T09:26:12Z

At the moment, the integration test is being run against cf-deployment_version: v40.15.0. We do not support modifications to feature flags, but I will discuss with the team to explore potential solutions.

One possible approach to support feature flags would be to introduce a file in the repository with a BOSH opsfile that could be applied prior to running tests. Please, let us know what you think about this approach.

Basic support for CNBs was released with v42.2.0, but it looks like our most recent changes are not released yet. Bumping the version now would not solve the issue, so I guess this will take a little longer.

Regarding the feature flag, we're using:

cli/.github/workflows/tests-integration-reusable.yml

Line 286 in a161328

cf enable-feature-flag diego_cnb
cli/integration/helpers/commonisolated/common_isolated_setup.go

Line 44 in a161328

helpers.EnableFeatureFlag("diego_cnb")

would that be enough once the integration tests consume the necessary cf-deployment version?

joaopapereira

In general looks good, but the tests are failing I am guessing it is because we changed the output. Can you please check that?
Added some other nit's related to formatting the code.

actor/v7pushaction/handle_cnb_credentials_override.go

actor/v7pushaction/handle_cnb_credentials_override_test.go

actor/v7pushaction/handle_lifecycle_override.go

command/v7/push_command.go

joaopapereira · 2024-08-19T15:05:36Z

command/v7/push_command.go

 	}, nil
 }

 func (cmd PushCommand) ValidateFlags() error {
 	switch {
+	// Lifecycle buildpack requested
+
+	case cmd.Lifecycle == constant.AppLifecycleTypeBuildpack && cmd.DockerImage.Path != "":


Since we are checking for dockerImage and docker-username, would it make sense to also test for docker-password?

Wrong input combinations, in regards to docker, are checked earlier. I guess it does not make sense to check it again.

command/v7/shared/app_summary_displayer.go

pbusko · 2024-09-09T12:48:42Z

@pbusko, pelase confirm if cf-deployment v40.15.0 with capi-release 1.182.0 be sufficient to verify these changes.

@a-b capi-release with the version >= 1.186.0 and the cf-deployment with the version >= v42.3.0 are required, together with the enabled diego_cnb feature flag

a-b · 2024-09-16T15:03:02Z

The cfd pool was updated to v40.15.0 last week. The shepherd team is working on the next upgrade.

a-b · 2024-09-16T15:03:26Z

@dependabot /rebase

a-b · 2024-09-16T20:11:43Z

Great news! Version cfd v43.3.0 is now available in the pool. Re-running integration tests.

a-b · 2024-09-18T01:47:57Z

Fixed environment deployment.
@pbusko, please rebase, and 🤞 this should get integration tests run.

a-b · 2024-09-18T13:49:56Z

It seems that both 1 and 2 integrations test failures are related to build packs and CNB:

  [FAIL] app command when the environment is set up correctly version independent display when the app exists when the app is a CNB app [It] displays the app buildpacks
  /__w/cli/cli/integration/v7/isolated/app_command_test.go:419

I also noticed the following error:

  ERR: CNBDetectFailed - cnb: detecting failed

during the push app step.

a-b · 2024-09-19T17:50:49Z

Checked both failed integrations tests 1, 2 locally. It works fine 🤷

a-b · 2024-09-20T21:29:31Z

I do not have write access to your branch, but essentially to fix tests, you want something like the following patch:

commit 54c988b260c238264014b1831c0563dccd25c5fc
Author: Al Berez <al.berez@broadcom.com>
Date:   Fri Sep 20 14:19:55 2024 -0700

    Fix push command help integration test

diff --git a/integration/v7/push/help_test.go b/integration/v7/push/help_test.go
index 328459207..7503e313a 100644
--- a/integration/v7/push/help_test.go
+++ b/integration/v7/push/help_test.go
@@ -26,6 +26,7 @@ var _ = Describe("help", func() {
 				"[-b BUILDPACK_NAME]",
 				"[-c COMMAND]",
 				"[-f MANIFEST_PATH | --no-manifest]",
+				"[--lifecycle (buildpack | docker | cnb)]",
 				"[--no-start]",
 				"[--no-wait]",
 				"[-i NUM_INSTANCES]",

a-b · 2024-09-20T21:33:47Z

Question (blocking): Do you have all help updates, like docker mode, covered?
Please double-check to see if you updated all help messages related to your changes.

a-b · 2024-09-20T21:38:43Z

Suggestion: To expedite interactions with approvers in the future, when creating a PR, please enable approvers to push changes to your branch by checking the corresponding checkbox in your PR.

a-b · 2024-09-23T13:39:34Z

@pbusko, @modulo11, I wanted to check if you need more help with this merge.

Co-authored-by: Johannes Dillmann <j.dillmann@sap.com> Co-authored-by: Pavel Busko <pavel.busko@sap.com> Co-authored-by: Nicolas Bender <nicolas.bender@sap.com>

Co-authored-by: Pavel Busko <pavel.busko@sap.com>

Co-authored-by: Pavel Busko <pavel.busko@sap.com> Co-authored-by: Nicolas Bender <nicolas.bender@sap.com>

Co-authored-by: Johannes Dillmann <j.dillmann@sap.com>

Co-authored-by: Pavel Busko <pavel.busko@sap.com>

Co-authored-by: João Pereira <joaopapereira@gmail.com>

Co-authored-by: Johannes Dillmann <j.dillmann@sap.com>

Co-authored-by: Pavel Busko <pavel.busko@sap.com>

pbusko · 2024-09-23T13:49:52Z

@a-b thank you for your help, we've updated the PR with your suggestion

a-b · 2024-09-23T16:29:13Z

Broken linter is a known issue that is is progress. I'll go ahead and merge, as integration tests and CATs are green.

pbusko force-pushed the cnb-lifecycle branch 2 times, most recently from 9cc994c to b967d16 Compare May 7, 2024 13:12

c0d1ngm0nk3y mentioned this pull request May 14, 2024

Add acceptance tests for cnb lifecycle cloudfoundry/cf-acceptance-tests#1136

Merged

7 tasks

pbusko force-pushed the cnb-lifecycle branch from b967d16 to 9feb622 Compare May 14, 2024 11:47

loewenstein mentioned this pull request Jun 7, 2024

Add cnb_app_lifecycle cloudfoundry/diego-release#929

Merged

1 task

loewenstein-sap mentioned this pull request Jun 12, 2024

[RFC0028] Implement Cloud Native Buildpacks lifecycle cloudfoundry/community#891

Open

pbusko force-pushed the cnb-lifecycle branch from 9feb622 to 924997e Compare June 19, 2024 10:49

pbusko marked this pull request as ready for review June 19, 2024 10:49

pbusko force-pushed the cnb-lifecycle branch 2 times, most recently from c6389b0 to ee145a4 Compare July 1, 2024 15:34

pbusko force-pushed the cnb-lifecycle branch from ee145a4 to 7cd1bac Compare July 23, 2024 06:22

pbusko force-pushed the cnb-lifecycle branch 3 times, most recently from 0bdafee to c1d9a1c Compare July 29, 2024 08:53

a-b reviewed Aug 2, 2024

View reviewed changes

pbusko force-pushed the cnb-lifecycle branch from c1d9a1c to d1aa343 Compare August 5, 2024 07:10

pbusko force-pushed the cnb-lifecycle branch 2 times, most recently from bc883c5 to b8b96d4 Compare August 12, 2024 07:22

pbusko force-pushed the cnb-lifecycle branch from b8b96d4 to 1d7feba Compare August 13, 2024 07:42

a-b approved these changes Aug 13, 2024

View reviewed changes

pbusko force-pushed the cnb-lifecycle branch 2 times, most recently from 27f04c8 to 8647baf Compare August 15, 2024 06:05

pbusko force-pushed the cnb-lifecycle branch from 8647baf to a161328 Compare August 19, 2024 09:15

joaopapereira requested changes Aug 19, 2024

View reviewed changes

pbusko force-pushed the cnb-lifecycle branch from 6e30e58 to ddc8a7c Compare September 9, 2024 12:48

pbusko requested a review from joaopapereira September 9, 2024 12:56

pbusko force-pushed the cnb-lifecycle branch from ddc8a7c to ecce4e1 Compare September 16, 2024 17:06

pbusko force-pushed the cnb-lifecycle branch from ecce4e1 to fc0894b Compare September 18, 2024 06:06

pbusko force-pushed the cnb-lifecycle branch from bd97558 to 2ae190c Compare September 19, 2024 13:10

pbusko force-pushed the cnb-lifecycle branch from 2ae190c to 5a1dbf8 Compare September 19, 2024 18:41

c0d1ngm0nk3y and others added 9 commits September 23, 2024 15:41

Add cnb lifecycle type

cae3f0f

Co-authored-by: Johannes Dillmann <j.dillmann@sap.com> Co-authored-by: Pavel Busko <pavel.busko@sap.com> Co-authored-by: Nicolas Bender <nicolas.bender@sap.com>

show buildpack summary for cnb lifecycle

8c1f409

Co-authored-by: Pavel Busko <pavel.busko@sap.com>

Add support for providing credentials for Cloud Native Buildpacks

76e4537

Co-authored-by: Pavel Busko <pavel.busko@sap.com> Co-authored-by: Nicolas Bender <nicolas.bender@sap.com>

Add cnb related integration tests and cats configuration

5c2db8a

Co-authored-by: Johannes Dillmann <j.dillmann@sap.com>

enable diego_cnb feature flag in integration tests

24f13a7

Co-authored-by: Pavel Busko <pavel.busko@sap.com>

Apply suggestions from code review

5e04e37

Co-authored-by: João Pereira <joaopapereira@gmail.com>

Fix test

93a4fb0

Co-authored-by: Johannes Dillmann <j.dillmann@sap.com>

use new JS hello world app for cnb integration

16ea1e7

Co-authored-by: Pavel Busko <pavel.busko@sap.com>

fix help usage test

db7b0f0

pbusko force-pushed the cnb-lifecycle branch from 5a1dbf8 to db7b0f0 Compare September 23, 2024 13:49

a-b merged commit 0cc96f4 into cloudfoundry:main Sep 23, 2024
16 of 17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC0028] Support for Cloud Native Buildpacks lifecycle #2871

[RFC0028] Support for Cloud Native Buildpacks lifecycle #2871

pbusko commented May 2, 2024 •

edited

Loading

loewenstein-sap commented Jul 23, 2024

beyhan commented Jul 23, 2024

a-b left a comment

pbusko commented Aug 5, 2024

a-b left a comment

modulo11 commented Aug 15, 2024

a-b commented Aug 16, 2024

modulo11 commented Aug 19, 2024

joaopapereira left a comment

joaopapereira Aug 19, 2024

modulo11 Aug 21, 2024

pbusko commented Sep 9, 2024

a-b commented Sep 16, 2024

a-b commented Sep 16, 2024

a-b commented Sep 16, 2024

a-b commented Sep 18, 2024 •

edited

Loading

a-b commented Sep 18, 2024

a-b commented Sep 19, 2024

a-b commented Sep 20, 2024

a-b commented Sep 20, 2024

a-b commented Sep 20, 2024

a-b commented Sep 23, 2024

pbusko commented Sep 23, 2024

a-b commented Sep 23, 2024

[RFC0028] Support for Cloud Native Buildpacks lifecycle #2871

[RFC0028] Support for Cloud Native Buildpacks lifecycle #2871

Conversation

pbusko commented May 2, 2024 • edited Loading

Where this PR should be backported?

Description of the Change

Why Is This PR Valuable?

Applicable Issues

How Urgent Is The Change?

Other Relevant Parties

loewenstein-sap commented Jul 23, 2024

beyhan commented Jul 23, 2024

a-b left a comment

Choose a reason for hiding this comment

pbusko commented Aug 5, 2024

a-b left a comment

Choose a reason for hiding this comment

modulo11 commented Aug 15, 2024

a-b commented Aug 16, 2024

modulo11 commented Aug 19, 2024

joaopapereira left a comment

Choose a reason for hiding this comment

joaopapereira Aug 19, 2024

Choose a reason for hiding this comment

modulo11 Aug 21, 2024

Choose a reason for hiding this comment

pbusko commented Sep 9, 2024

a-b commented Sep 16, 2024

a-b commented Sep 16, 2024

a-b commented Sep 16, 2024

a-b commented Sep 18, 2024 • edited Loading

a-b commented Sep 18, 2024

a-b commented Sep 19, 2024

a-b commented Sep 20, 2024

a-b commented Sep 20, 2024

a-b commented Sep 20, 2024

a-b commented Sep 23, 2024

pbusko commented Sep 23, 2024

a-b commented Sep 23, 2024

pbusko commented May 2, 2024 •

edited

Loading

a-b commented Sep 18, 2024 •

edited

Loading