-
Notifications
You must be signed in to change notification settings - Fork 7
Adds API specification for the provider service. #28
base: master
Are you sure you want to change the base?
Conversation
dadbf50
to
75fa7dd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi! I've identified the causes of the validation failure.
98c514b
to
d34dd55
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi! A few comments. Let me know what you think!
docs/ProviderDesign.md
Outdated
a Contract will be made up of following attributes: | ||
|
||
* Contract ID: Unique ID per contract | ||
* List of Offers: The exact nodes that will participate in serving this contract can be inferred from the list of offers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which offers? A marketplace offer and a provider offer will have different offer IDs, correct? Which brings up the question - will marketplace offers and provider offers also track the other's offer IDs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A contract is created by the marketplace. The matching engine does not have access to the data of the provider service. It can only match the offers available in the marketplace. Therefore the ids mentioned in the list of offers in the contract will be ids assigned by the marketplace service when accepting the offers from the provider service. Provider service maintains its own UUID for each offer, but that information is private to the provider service.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the provider contract however, correct? I think it would be odd for it to refer to the marketplace offer rather than the offer that the provider knows about.
2. Provider service confirms owner-hardware relationship before accepting offers. | ||
3. For each accepted offer it fetches the hardware configuration from Ironic | ||
4. These offers are published to the marketplace as soon as possible. | ||
5. Receives contracts from the marketplace. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we talked about this, I think it was decided that the provider pulled contracts from the marketplace. Is that still correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part is not clear in my understanding. Once a contract is made, who executes it, provider service or the hardware owner or some other OpenStack service? Because setting up the contract requires providing necessary networking also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The provider service has to be able to manage contracts on its side; this way it can still expire contracts if it gets disconnected from the marketplace
|
||
* Node Configuration: (as extracted from Ironic) | ||
|
||
## Offers (Providers --> Marketplace): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this section, as marketplace offers are fully described in the marketplace doc. It may be enough to just mention that a provider's offer will be sent to the marketplace, alongside any ironic info.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was put here just to show the additional attributes that a published offer contains as compared to the one that is just submitted to the provider service and not yet published.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to link to the marketplace documentation; duplicating information in two different places is a good way for them to eventually go out of sync.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
understood. will link it.
* InContract: When an offer finds a suitable bid and a contract is formed. | ||
* Expired: For any reason if the offer is cancelled or gets past its time of availability or gets used. | ||
|
||
Provider also fetches the node configuration from Ironic and adds it to the offer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is quite right; this information isn't added to the provider's offer. Rather, when the offer is submitted to the marketplace, the Ironic information is submitted as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The context is what actions does the provider service take on receiving the offer from the owner.
it
- verifies if the owner is authorized to make an offer
- extracts hw_config from ironic and appends it to the offer
- Publishes the offer to the marketplace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"appends it to the offer" implies that the provider offer has the ironic data added to it (at least it does to me). I'd argue that's not really what's happening. Instead, the provider uses the provider offer and the ironic information to create a marketplace offer.
d34dd55
to
6571d36
Compare
An offer made by an owner and submitted to the provider service will have following attributes. | ||
* Node ID: Unique ID of the node | ||
* Start time: Date and time of availability | ||
* Duration: Hours, days that the node will be available |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure we had settled on offers having a start time and end time, not a start time and duration: https://github.com/CCI-MOC/flocx/blob/master/docs/specs/marketplace-api.yaml#L700
* Duration: Hours, days that the node will be available | ||
* Floor Price: Minimum Price that is demanded. | ||
|
||
Provider validates the offer received from the owner, i.e. verifies that the owner matches the once listed for the node. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The phrasing here is unclear..."matches the once list for the node"?
* List of Offers: The exact nodes that will participate in serving this contract can be inferred | ||
from the list of offers. | ||
* Start Time: Contract commencement (date and time) | ||
* Duration: Time span the contract lasts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
End time, not duration.
* Start Time: Contract commencement (date and time) | ||
* Duration: Time span the contract lasts | ||
* Price: Price at which the contract was made. | ||
* properties: This defines additional properties relevant to the contract which are not intrinsic to the resource itself. One example is contract_callback_url. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is contract_callback_url
? Where is it defined? It doesn't show up in the marketplace api specification.
example: | ||
status: 'Error' | ||
message: 'offer id does not exist' | ||
put: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need an API call for updating the status? In what situations will this be used? Shouldn't the status be managed exclusively by the workflow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a few comments inline.
There are still some review comments here that need to be addressed. This PR will also need to be rebased against the changes introduced in #35.
No description provided.