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.

Sign up for GitHub

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

Jump to bottom

Simplify restartPolicy: change to string #3607

Closed
abonas opened this issue Jan 19, 2015 · 8 comments · Fixed by #5477
Closed

Simplify restartPolicy: change to string #3607

abonas opened this issue Jan 19, 2015 · 8 comments · Fixed by #5477
Assignees
@dchen1107
Labels
area/api Indicates an issue on api area. area/usability priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.
Milestone
v1.0

Comments

@abonas
Copy link
Contributor

abonas commented Jan 19, 2015

In the pods in both v1 and v3 api, the restart policy looks like this:

"restartPolicy": {
"always": {}
}

Why is this a hash inside hash? Shouldn't it be a simple key "restartPolicy" with string value one of: RestartPolicyAlways, RestartPolicyOnFailure, RestartPolicyNever ?
@thockin
Copy link
Member

thockin commented Jan 20, 2015

The assumption was that the various policies would soon grow parameters,
which would live in those inner objects. Given that this has not happened
yet, maybe we should reconsider this design for v1beta3?

@dchen1107 - will we grow params here? E.g. max number of restarts or
specific failure types?

On Mon, Jan 19, 2015 at 2:57 AM, abonas notifications@github.com wrote:

In the pods in both v1 and v3 api, the restart policy looks like this:

"restartPolicy": {
"always": {}
}

Why is this a hash inside hash? Shouldn't it be a simple key
"restartPolicy" with string value one of: RestartPolicyAlways,
RestartPolicyOnFailure, RestartPolicyNever ?

Reply to this email directly or view it on GitHub
#3607.

@bgrant0607 bgrant0607 added area/api Indicates an issue on api area. kind/documentation Categorizes issue or PR as related to documentation. area/usability priority/backlog Higher priority than priority/awaiting-more-evidence. labels Jan 24, 2015
@bgrant0607
Copy link
Member

@thockin : My answer is no, we will not expand these policies.

@bgrant0607 bgrant0607 removed the kind/documentation Categorizes issue or PR as related to documentation. label Jan 24, 2015
@bgrant0607
Copy link
Member

I'd be happy to have this simplified in v1beta3.

@thockin
Copy link
Member

thockin commented Jan 24, 2015

Really? You know the 800 knobs that we have internally - none of them are
useful?

I'd be happy to see this simplify to a string like the other *Policy
fields, but the conversions and testing will have to be written carefully
(as I discovered this week :)

On Fri, Jan 23, 2015 at 5:23 PM, Brian Grant notifications@github.com
wrote:

@thockin https://github.com/thockin : My answer is no, we will not
expand these policies.

Reply to this email directly or view it on GitHub
#3607 (comment)
.

@bgrant0607
Copy link
Member

Really.

@dchen1107 dchen1107 self-assigned this Jan 24, 2015
@dchen1107
Copy link
Member

Ok, I will clean up this one next week. And we can add knobs later if someone come up with real good reasons.

@bgrant0607 bgrant0607 added sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. and removed sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. labels Feb 5, 2015
@bgrant0607 bgrant0607 modified the milestone: v1.0 Feb 6, 2015
@bgrant0607 bgrant0607 added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed priority/backlog Higher priority than priority/awaiting-more-evidence. labels Feb 28, 2015
@bgrant0607 bgrant0607 changed the title REST api - clarification needed for restartPolicy format Simplify restartPolicy: change to string Feb 28, 2015
@bgrant0607
Copy link
Member

I'd like to get this into v1beta3 asap.

@dchen1107 dchen1107 self-assigned this Mar 3, 2015
@dchen1107
Copy link
Member

@yujuhong is working on #4623, and there might be some overlaps between two issues.

@bgrant0607 bgrant0607 mentioned this issue Mar 10, 2015
Closed
dchen1107 added a commit to dchen1107/kubernetes-1 that referenced this issue Mar 14, 2015
@dchen1107
Convert RestartPolicy to string for v1beta3.
34e9c82 
Fixed kubernetes#3607 and spiritually support kubernetes#5475
@dchen1107 dchen1107 mentioned this issue Mar 14, 2015
Merged
@simon3z simon3z mentioned this issue Mar 25, 2015
Merged
simon3z added a commit to simon3z/manageiq that referenced this issue Mar 25, 2015
@simon3z
kubernetes: handle new pod restartPolicy format
4ba239f 
This patch adds the support of the new restartPolicy format:

  kubernetes/kubernetes#3607

Until we're sure that everyone updated their kubernetes instance
(probably until v1.0 is officially released) it's better to handle
both.

Signed-off-by: Federico Simoncelli <fsimonce@redhat.com>
akram referenced this issue in akram/kubernetes Apr 7, 2015
@dchen1107 @akram
Convert RestartPolicy to string for v1beta3.
00b6f24 
Fixed #3607 and spiritually support kubernetes#5475
@snyk-bot snyk-bot mentioned this issue Jan 14, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dchen1107 dchen1107

Labels
area/api Indicates an issue on api area. area/usability priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.
Projects
None yet
Milestone
v1.0
Development

Successfully merging a pull request may close this issue.

Convert RestartPolicy to string for v1beta3. dchen1107/kubernetes-1
4 participants
@thockin @abonas @bgrant0607 @dchen1107