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

Implement Manifest V3 objects and deployment #1167

Merged
merged 280 commits into from
Nov 1, 2022

Conversation

schulzh
Copy link
Contributor

@schulzh schulzh commented Sep 16, 2022

Implements:

  • objects and logic required for reading and writing CC API V3 manifests
  • client interface and reactor implementation to apply the manifest
  • resource matching for v3 packages API
  • pushManifestV3 operation, similar to the existing pushManifest operation

I hope that the pushManifestV3 operation is complete (especially the "app readiness" checks), since I did not find a documentation of this process and had to reverse engineer it from the CLI.

I chose to pass a raw byte array as manifest in the client instead of an object, because that would've required to add the snakeyaml dependency to the client as well. Since the manifest is just passed to and applied by the CC anyway, I saw no benefit of passing as an object.

Unfortunately, I could not get a local CF instance set up for the integration tests. Due to licensing issues at my company I cannot use Virtualbox and setting it up on a local OpenStack "DevStack" instance wasn't working either. If requested, I can add integration tests, but I cannot ensure that they are working as intended.

Solves #999 and #1142 but not #1103 yet

twoseat and others added 30 commits May 28, 2020 10:52
This commit adds support for maintenance info when updating service instances

Signed-off-by: Paul Harris <harrisp@vmware.com>
Signed-off-by: Paul Harris <harrisp@vmware.com>
This commit adds integration tests for Audit Events, and handles issues uncovered by the
tests. It also adds some missing javadoc, and tweaks some naming and visibilities.

[resolves cloudfoundry#1014]

Signed-off-by: Paul Harris <harrisp@vmware.com>
Earlier versions of the client would automatically refresh an expired refresh token using
the credentials it already had. This functionality was lost during the transition to newer
versions of reactor. This commit reinstates that functionality.

[resolves cloudfoundry#1051]

Signed-off-by: Paul Harris <harrisp@vmware.com>
Adds fields required for compatibility with PCF 2.9, and update a potentially fragile test

Signed-off-by: Paul Harris <harrisp@vmware.com>
Previously the audit events space and organization relationships were defined as custom
objects, when they should have used the standard Relationships object. This commit
corrects that error.

Signed-off-by: Paul Harris <harrisp@vmware.com>
This commit adds logging of the X-Vcap-Request-Id header on responses to aid diagnosing
communication issues with a cloud foundry server. The added information is available by
enabling TRACE logging, minimally on cloudfoundry-client.response.

[resolves cloudfoundry#981]

Signed-off-by: Paul Harris <harrisp@vmware.com>
The project uses two word lists to generate random routes. These words have zero meaning
within the project, they are effectively random strings that can be pronounced. Because
they have no meaning to the project there is no reason for any even theoretically
objectionable word to appear in the lists. Therefore this commit removes a non-exhaustive
selection of such words.

Signed-off-by: Paul Harris <harrisp@vmware.com>
This commit adds support for maintenance info at the operations layer, allowing
service instances to be upgraded to new versions of a service.

Signed-off-by: Paul Harris <harrisp@vmware.com>

[resolves cloudfoundry#1050] [resolves cloudfoundry#1052]
luschmar and others added 9 commits June 1, 2022 12:16
)

* Add variable substitution in ApplicationManifestUtils
    * Sanitize regex input
    * Throw exception on missing variable substitution
    * Add test for variable substitution in manifest
…stance to complete (cloudfoundry#1161)

The update test does two things: 1.) it creates a managed service instance and 2.) it updates the managed service instance. The second part was failing because CloudController will refuse to update a service that's still being created.

The update job was calling a method `waitForCompletionOnCreate` which was looking at the CreateServiceInstanceResponse object to see if it had an job id, if it did, then it would call `JobUtils.waitForCompletion(cloudFoundryClient, Duration.ofMinutes(1), createServiceInstanceResponse.getJobId().get())` and then return `getServiceInstanceIdByName(cloudFoundryClient, serviceInstanceName)`. The problem is that the call to `JobUtils.waitForCompletion` was creating a flow but nothing was subscribing to it, so it never actually ran and the test would not pause until the creation finished.

The test has been modified such that it assumes the creation request will be asynch and return a job id. If it does not for some reason, the test would fail as wel call `.get()` on an optional object and calling that if the optional is empty will generate a NoSuchElementException. Then with the jobId, we call `JobUtils.waitForCompletion` wrapped in a flatMap, which ensures it's part of the overall flow which is being subscribed to.

The same change was made after the update request, which also returns an asynch response and needs to be polled.

Signed-off-by: Daniel Mikusa <dmikusa@vmware.com>
There are presently three LifecycleTypes supported: buildpack, docker and kpack. The first two have been supported, and this PR adds support for the third. It also includes a KpackData class for the lifecycle data that the kpack lifecycle type can return.

API documentation clarifies what is allowed:

http://v3-apidocs.cloudfoundry.org/version/3.122.0/index.html#lifecycles

Includes a test which expands and deserializes the Kpack LifecycleType.

Resolves cloudfoundry#1094

Signed-off-by: Daniel Mikusa <dmikusa@vmware.com>
Fix service credential binding details response
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 16, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: schulzh / name: Hans Schulz (e883106)

@pivotal-david-osullivan
Copy link
Contributor

Thanks for the PR @schulzh! Are you able to follow the link in the EasyCLA comment above to get the commit authorised
✅ and satisfy the bot?

SInce this is a significant addition, if you can add the integration tests I am happy to run them in a lab!

@schulzh
Copy link
Contributor Author

schulzh commented Sep 16, 2022

I have already done the EasyCLA process and I'm waiting on the approval of my organization. I'll add integration tests then, thanks!

@schulzh
Copy link
Contributor Author

schulzh commented Sep 21, 2022

Hi @pivotal-david-osullivan,

I've added integration tests. Could you please run them in your lab?
I've also completed the CLA. From my side this PR should be ready to merge, so I'd be happy to have it reviewed.

@pivotal-david-osullivan pivotal-david-osullivan added triaged Initial triage of issue has been performed enhancement Improvement or new functionality labels Sep 23, 2022
Copy link
Contributor

@mheath mheath left a comment

Choose a reason for hiding this comment

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

This is an amazing contribution. Thank you thank you thank you for all your hard work.

There's a fix that needs to happen in v3/SpacesTest.java (see the comments there)

Other than that, most of the rest of my comments are nitpicks about method order.

Once you fix these things, I'll merge this into its own branch so that I can write acceptance tests for it. Once we have tests, we'll merge this into main.

private Mono<String> spaceId;

//TODO how to check if resource matching is enabled on this CF instance?
@IfCloudFoundryVersion(greaterThanOrEqualTo = CloudFoundryVersion.UNSPECIFIED) //TODO how to select this version?
Copy link
Contributor

Choose a reason for hiding this comment

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

I will look into this

@schulzh
Copy link
Contributor Author

schulzh commented Oct 6, 2022

Hi @mheath ,

Thank you for the review and kind words 🙂

I've addressed the issues, please take a look.

@mheath
Copy link
Contributor

mheath commented Oct 21, 2022

Will you change the branch from main to manifest-v3 on this PR and I will get it merged and then merge in my acceptance tests?

@schulzh schulzh changed the base branch from main to manifest-v3 October 21, 2022 21:31
@schulzh
Copy link
Contributor Author

schulzh commented Oct 22, 2022

Hi @mheath,

I've changed the base branch. It looks like its lagging behind main a few hounded commits. Should I rebase it?

@mheath mheath merged commit 81f8eb9 into cloudfoundry:manifest-v3 Nov 1, 2022
@schulzh
Copy link
Contributor Author

schulzh commented Jan 17, 2023

Hello @mheath,

do you have any updates on this? What is missing to get it merged to master?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement or new functionality triaged Initial triage of issue has been performed
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.