Integration Lifecycle Handling

syndesis-bot commented 6 years ago

@rhuss	2017-06-16	Epic, Feature, Runtime

At the moment, when integrations are created they have completely disconnected from the Syndesis DB afterwards. There is no way yet to update a running integration nor see the state of the currently running integration.

Therefore it is suggested to introduce a versioning for integration and rethink the state model with "active" being a valid state.

The following points should be considered:

Multiple versions of an Integration must be able to exist.
Exactly one of these integrations can be active.
Each deployment of an Integration creates a new version.
Users can switch integration versions from the UI
Versioned Integrations can be inspected from a history page.
Old integrations can't be changed but can be used as a blueprint for new integrations.
As soon as an integration is changed, a new version of an integration is created. All changes go to this new integration until it is deployed by the user. After this, its gets a fixed version and cannot be changed anymore.
The user must always be able to see and examine the currently active integration along with its version number.

@kcbabo @chirino I really would like to discuss this lifecycle handling of integrations a bit in more detail. I'm sure that our simplistic handling of integration versions is not sufficient (even not for PR1)

syndesis-bot commented 6 years ago

@jimmidyson	2017-06-26

I wonder how much we can piggy back off of deployment revisions here? I guess we need to version the integration in the DB though, not keen on doing that tbh but can't see a way round it.

Just to get things moving with this, how about a model something like:

type Integration {
 Spec IntegrationSpec // The user selected integration spec (reconciled against Status.ActiveSpec to look for changes)
 SpecVersions IntegrationSpec[] // Array of versions, ordered by age, oldest first. Integrations can be deleted from anywhere in this array, unless it is the Status.ActiveSpec

 Status IntegrationStatus // As we talked about 
}

// IntegrationSpec should be immutable, editing an IntegrationSpec should create append a new revision to Integration.SpecVersions
type IntegrationSpec {
 ... Current spec fields...
}

type IntegrationStatus {
 ActiveSpec IntegrationSpec // The currently active integration spec

 ...Other status fields...
}

As IntegrationSpec is immutable, we could also hash the IntegrationSpec and use the hash in IntegrationStatus.ActiveSpec and store the hash in IntegrationSpec.Hash potentially?

syndesis-bot commented 6 years ago

@rhuss	2017-06-26

So iiur then Integration.Spec is the desired spec and Integration.Status.ActiveSpec the current spec.

Wouldn't it make sense to reconcile on a whole status as it also there is a target status (like 'running', 'stopped', '...') which should be reconciled ? Or is this part of the spec ?

Would it make sense to include a version number within the IntegrationSpec so that the comparison between targetState and currentState is easier ? Also to see what is older, what is younger ? (not so easy with a hash).

Maybe we just call it Integration.TargetSpec to make it clearer and more symmetric to ActiveSpec.

Whatever, we should start with some design proposal which is important as the state model and the versioning is really at the heart of Syndesis. I'd pretty happy to copycat K8s Deployment model wrt to status / versioning.

syndesis-bot commented 6 years ago

@jludvice	2017-06-28

Did we consider various race condition situations ?

developer doing git push with his changes
syndesis doing
- save integration > push changes - we may expect conflicts here
- opening existing integration and loading changes can we get conflicts here?
- pulling changes into integration pod probably no conflicts here, right?
two citizen developers editing the same integration at the same time

Assuming expert developer makes some changes directly to git, how will he trigger redeploy to verify his changes?

syndesis-bot commented 6 years ago

@chirino	2017-06-28

@rhuss you could use the KeyGenerator to generate your version ids. The keys will be unique and always in increasing order (it has a timestamp component to it).

syndesis-bot commented 6 years ago

@rhuss	2017-06-28

@chirino although this might help it would be awesome to go exactly to the last version by decrementing the version number by 1 (or any calculation without looking up all versions). The version needs to be unique for this specific integration only, so a plain counter is completely sufficient.

syndesis-bot commented 6 years ago

@rhuss	2017-06-28

@jludvice these are all good points. I wonder whether we should avoid merging at all and use orthogonal branches for the integration (so each deployment of an integration creates a new branch corresponding to the version number). In that case, we don't have issues with merging but still can get back and forth by switching branches.

The interaction with the git repository happens only when an integration gets deployed/redeployed, which is an atomic operation (i.e. should be locked somehow so that only one deployment at a time is allowed).

For the expert user to work directly with git I would say this is not a valid use case for now, but theoretically, he could change data on a branch and hence modifying a current version. For this flow to work (i.e. from git to syndesis db) a lot more things would have to be considered to get changes into the DB.

For now we should only consider one direction: From Syndesis -> Git.

syndesis-bot commented 6 years ago

@jludvice	2017-06-29

@rhuss I like the idea of decrementing by one to get previous version

Regarding "Exactly one of these integrations can be active." It's common to have several envs like dev >> test >> stage >> production right? Should we have concept of envs in ipaas? Could we eventually implement that with "hard lock" on one instance? Would it be easier to use Copy+Paste to create devel version of integration?

As a citizen user I probably won't have a glimpse of dev >> stage>> production envs, but having a way of "playing harmlessly" until it's ready is easy to understand. I could make dozens of attempts until I do data mapping and stuff right and I might need do replace certain connections with their test/mock counterparts.

I understand it's a big thing to implement. Can we manage to implement this issue in a way that won't close a door for requirements like this in the future?

syndesis-bot commented 6 years ago

@dongniwang	2017-06-30

Is there a UX design need down the road for this?

syndesis-bot commented 6 years ago

@rhuss	2017-07-03

@dongniwang @jludvice @kcbabo I think we need some initial discussion what exactly of this kind of lifecycle management affects the UI and also which parts of the UI. So at the moment the answer is probably "yes, there is, but we still have to check which".

syndesis-bot commented 6 years ago

@jludvice	2017-07-10

@rhuss regarding the integration state model,

suspend/resume should be swapped
why do we only allow deleting of drafts? does it mean I can't delete some old revision of integration?
what if I create integration which crashes / goes to error state and I have no idea how to fix it? How do I delete it to start fresh?

anyway +1 for nice graph ;)

syndesis-bot commented 6 years ago

@rhuss	2017-07-10

Indeed suspend / resume is wrongly placed. Thx !
That's a good question whether we want allow undeployed integrations to be deleted. Depends of course on the business case, so it's up to @kcbabo to decide (one would loose auditing history, but I have no strong feelings here :).
For the error case the 'fix' can be of different nature:
- Fixed automatically because it was only a temporary error (e.g. in the backend)
- Some action required. This is individual to every error kind and there should be an indication e.g. in the error message how it can be fixed.
- Disable the integration by moving it to "undeployed". Also I wonder, whether an Inactive integration can move to Error, but that might happen when one tries t "Resume" ? Same for "Undeployed" when doing a "Reactivate" ? wdyt ?

syndesis-bot commented 6 years ago

@rhuss	2017-07-10

Graph is btw created via plantuml

syndesis-bot commented 6 years ago

@rhuss	2017-07-11

Current status: Design Proposal parts for the backend are finished, the UI is not yet perfectly shaped out. Next steps: Change backend to reflect the state model, create UX design for the integration overview.

syndesis-bot commented 6 years ago

@sjcox-rh	2017-07-20

Updates to the integration lifecycle designs can be found here:

https://github.com/syndesisio/syndesis-ux/pull/14/

FYI: @chirino @jimmidyson @jludvice @iocanel

syndesis-bot commented 6 years ago

@jimmidyson	2017-09-14

Going to move this from sprint 17. We now have history without sweeping API changes thanks to syndesisio/syndesis-rest#593 and we can relook at this in future sprints.

syndesis-bot commented 6 years ago

@dsimansk	2017-10-30

@rhuss after a while of using resource constrained project on Online cluster. I think we need a way to gather exceeding events from OpenShift and report them back to user in a simple && meaningful way. E.g. permanent warning banner similar to OS console, that quota is exceeded etc.

This seems like a complex task(backend,ui,uxd) partially related to lifecycle, but should I open new issue for it? Wdyt?

syndesis-bot commented 6 years ago

@rhuss	2017-10-30

@dsimansk Yes, I think this a good idea to present any resource constraints like this, possibly with a link to further details and how to get around it.

if you could open an issue in syndesis-project that would be awesome. I think we already have an issue which is about an UI integration of the limit restrictions, maybe you can look to this, too ?

syndesis-bot commented 6 years ago

@kcbabo	2017-10-30

@sjcox-rh @dongniwang @amysueg Note the request for a notification when exceeding resource limits.

syndesis-bot commented 6 years ago

@amysueg	2017-10-31

@kcbabo @dongniwang @sjcox-rh @dsimansk @rhuss See my comments on #175

zregvart commented 6 years ago

I believe this is out of scope for TP3 and we want to target this for TP4.

gashcrumb commented 6 years ago

@iocanel couple questions on the behaviour now that I've finally gotten a look at it (don't ask :-p ). First off I did the following steps:

1) Create an integration called test1 with the usual twitter to salesforce flow and published it 2) Added a basic filter and published that too 3) Renamed the integration to test2 4) Added an advanced filter but only saved that change as a draft.

I've noticed some confusing things though.

1) All 3 deployments (when I hit either /integrations/{id}/history or /integrations/{id}/deployments show the current and target states as Active. Surely only the running one should be active? 2) The deploymentId is set to 3, there's no deployment with that ID. I guess that's what indicates the value from /integrations/{id} is a draft? 3) Since I have a draft but I also have an active integration how do I tell what deployment is actually active? The integration object points to a non-existent deployment, do I just assume the highest ID then? 4) The latest deployment doesn't have the updated name, it still refers to test1.

Also, am not sure when I'd use the history endpoint vs. the deployments endpoint, seems like I only really need one of these to render this table.

I was going to create gists with the json responses but they've got my twitter/salesforce API keys in 'em in multiple places, so no gist for you :-) sorry!

iocanel commented 6 years ago

@gashcrumb: These sound like bugs. Let me have a look.

Regarding the history vs deployments. The history is supposed to also contain messages, which at the moment I don't recall how to fill. @chirino: Can you please shed some like on what the messages of the IntegrationHistory (formerly named IntegrationStatus) should contain?

gashcrumb commented 6 years ago

Awesome, thanks! One thing that I'm finding difficult when looking at the designs is handling that 'draft' area with the data I'm getting back. Seems like maybe 'draft' shouldn't necessarily be a state enum value but really just a flag that the integration has changes as compared to the current running instance? 'Cause like I'd never expect to see a deployment there with a Draft state for example, you know?

chirino commented 6 years ago

@iocanel @gashcrumb

I think we were supposed to have 1 more API view class to hold overall integration state. Think of it as returning everything the Syndesis detail page needs to be rendered to the user. Perhaps we should call it IntegrationDetails :)

It wold have a field for the Integration, a list for the Deployments, and any upgrade/validation messages associated with the integration. We could also add in the metrics data for the integration here too.

chirino commented 6 years ago

But perhapas we just let the UI do multiple API calls to collect all those bits. I know for the upgrade and data import/export epics we need an API to get those upgrade/validation messages associated with an integration/connection.

gashcrumb commented 6 years ago

I think we were supposed to have 1 more API view class to hold overall integration state. Think of it as returning everything the Syndesis detail page needs to be rendered to the user. Perhaps we should call it IntegrationDetails :)

Yeah, this sounds more straightforward.

Multiple API calls is probably fine too at least for a first pass, I'm planning on splitting out the history/draft bits into separate components anyways, so these guys can handle making those calls.

I still find the Draft status kind of an issue and kinda think it shouldn't be a state but a separate flag, though then again if it's never been deployed... I dunno :-(

iocanel commented 6 years ago

There reason it needs to be a state is that its something that the user can explicitly select by just saving an integration without publishing it.

gashcrumb commented 6 years ago

So really then the component that's supposed to reflect the integration state should generally always be looking at whatever the active deployment is, not whatever fields are in the response to /integrations/{id}. I think that's certainly doable but does complicate fetching the data if it takes multiple requests, as for the list to accurately reflect state it would also have to go and fetch the deployments, not so bad for the detail page but for the list page that's not ideal. Maybe instead of referring to the active deployment via ID we could just embed that in the integration object?

chirino commented 6 years ago

iocanel, perhaps we should make publishing an separate API call.

iocanel commented 6 years ago

@gashcrumb, @chirino: I am experimenting doing that on the backend, so that its transparent to the ui. Would work for you?

gashcrumb commented 6 years ago

Could also decouple the draft vs not draft from the runtime state, it's kinda like we're trying to capture 2 different things in one field.

gashcrumb commented 6 years ago

@iocanel yeah, lemme know how it goes, I can always check out your branch and give it a go here

iocanel commented 6 years ago

Yeah, it seems that in the backend Draft has two major uses:

as desired state: save but not publish
as current state: activating but not active yet

The later should probably be replaced by pending.

Do you have other uses of Draft that I missed?

chirino commented 6 years ago

@iocanel agree /w using pending for that.

iocanel commented 6 years ago

So both using pending and also handling the current deployment status in the backed seem to work really nice. Running some last tests and will push my changes.

iocanel commented 6 years ago

@chirino: earlier I asked about the messages in the IntegrationHistory object (formerly known as IntegrationStatus). At the moment they are blank. What kind of information should be shown in that list?

chirino commented 6 years ago

@iocanel The upgrade epic will likely figure out what the structure of messages needs to be. Perhaps we make it 2 lists:

 List<ValidationErrors> validations;
 List<UpgradesAvailable> upgrades;

tplevko commented 6 years ago

I have noticed several issues with current lifecycle handling: 1578, 1558 and 1559. The most serious one seems to be related to versioning, which is not working as expected ATM. I wanted to ask, whether this epic should be marked as "done", as there are these serious issues.

iocanel commented 6 years ago

On 02.15.18, Tomas Plevko wrote:

I have noticed several issues with current lifecycle handling: 1578, 1558 and 1559. The most serious one seems to be related to versioning, which is not working as expected ATM. I wanted to ask, whether this epic should be marked as "done", as there are these serious issues.

Let me add the following issues to the mix: https://github.com/syndesisio/syndesis/issues/1580 https://github.com/syndesisio/syndesis/issues/1581

And no, we can't mark it as done before we address at least the most serious ones.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/syndesisio/syndesis/issues/184#issuecomment-365952005

syndesisio / syndesis

Integration Lifecycle Handling #184