-
Notifications
You must be signed in to change notification settings - Fork 249
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
be explicit about ports for workers and servers #217
be explicit about ports for workers and servers #217
Conversation
Signed-off-by: Matt Butcher <[email protected]>
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.
lgtm
Signed-off-by: Matt Butcher <[email protected]>
Closes #182 |
3.component_model.md
Outdated
| `gpu` | [`GPU`](#gpu) | N | | Specifies the attributes of the gpu resources required for the container. | | ||
| `volumes` | [`[]Volume`](#volume) | N | | Specifies the attributes of the volumes that the container uses. | | ||
| `extended` | [`[]ExtendedResource`](#extendedresource) | N | | Implementation-specific extended resource requirements | | ||
|
||
For any resource that cannot be satisfied by the underlying platform, the platform MUST return an error and cease deployment. A resource is considered a requirement, and failure to meet that requirement means the runtime MUST NOT deploy the application. For example, if an application requests `1P` of memory, and that amount of memory is not available, the application deployment must fail. Likewise, if an application requires `1` gpu, and the runtime does not provide gpus, the application deployment MUST fail. | ||
|
||
If resources are not specified, the underlying platform is free to choose a suitable default. |
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 this might cause weird & hard to debug errors. Is there a specific reason for making this to be not required?
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.
Yes -- issue #182
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.
And this was complicated by the fact that the way different platforms measure CPU and memory turn out to be different, as we discovered on Kubernetes a few weeks back.
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.
Or maybe I should ask. In Kubernetes, a sensible default for CPU is somewhere around 0.05
and maybe 0.1
, because in Kubernetes, this is how much of a Kubernetes Node's actual CPU you need. I am assuming that for SF, the sane default is 1
.
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.
We can do fractional cores as well, but I am thinking of cases where the default causes activation of the container to fail and makes it hard and not obvious for the operator to debug(we have seen such cases when we were doing SF Mesh).
Also, the issue #182 mentions Resources are optional for containers, they are mandatory as well in the current schema(is that comment meaning different resources than this one below?).
resources | Resources | Y | | Resources required by the container.-- | -- | -- | -- | --
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.
Removed that section from this PR, opening another PR to deal with just #182
42da44f
to
a788271
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.
Closes #199
Signed-off-by: Matt Butcher [email protected]