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

NFD-Topology updater as a oneshot job #600

Closed
swatisehgal opened this issue Sep 17, 2021 · 7 comments · Fixed by #622
Closed

NFD-Topology updater as a oneshot job #600

swatisehgal opened this issue Sep 17, 2021 · 7 comments · Fixed by #622
Labels
Hacktoberfest kind/feature Categorizes issue or PR as related to a new feature.
Milestone

Comments

@swatisehgal
Copy link
Contributor

swatisehgal commented Sep 17, 2021

As a follow-up from the discussion in #525, it has emerged that we should reconsider deploying nfd-topologyupdater as a oneshot job. We should do one of the following:

  1. Deploy nfd-topologyupdater only as a daemonset and remove its ability to be deployed as a oneshot job
  2. Populate only capacity and allocatable (and not available) when --oneshot flag is specified
  3. just simply log in case --oneshot and not create NRT CRs
@marquiz
Copy link
Contributor

marquiz commented Oct 4, 2021

I think we could do # 2 above. I.e. only update node capacity and allocatable, and not scan pods at all if --oneshot is specified.

WDYT?

@marquiz marquiz added Hacktoberfest kind/feature Categorizes issue or PR as related to a new feature. labels Oct 4, 2021
@zwpaper
Copy link
Member

zwpaper commented Oct 5, 2021

hi @marquiz, I would like to take action on this issue, but I not yet figure out what we should do exactly, should we

  1. update the kustomization leaving NRT daemonset only
  2. do nothing in NRT cmd as daemonset has an Always restart policy
  3. only update node capacity and allocatable, and not scan pods in NRT

@marquiz
Copy link
Contributor

marquiz commented Oct 5, 2021

Hi @zwpaper, thanks for reaching out!

A good question, indeed 😇 Above, I was pondering that only publishing node capacity and allocatable would be a good solution but now I'm starting to think otherwise.

  1. update the kustomization leaving NRT daemonset only

I'm now inclined towards this solution, drop the Job deployment alltogether. The problem I see with Job is that all the information is dynamic, extended resources can be added/removed etc. Also, the users should be able to get capacity/allocatable by examining the node status directly. We can leave the --oneshot parameter so that users can deploy it as a job if they want (plus I think it is probably useful for testing/development/debugging, too). But, I think we probably shouldn't advertise as a recommended/supported deployment option.

@swatisehgal any thoughts?

@swatisehgal
Copy link
Contributor Author

I am okay with removing the topology-updater job overlay and leaving the --oneshot parameter. I think it hits the sweet spot as it gives the ability to the user to deploy topologyupdater as a job (if they really want to) for inspection/testing/development/debugging and therefore naturally comes with consciousness that it is being deployed as oneshot job and should be considered static information.

@marquiz
Copy link
Contributor

marquiz commented Oct 5, 2021

@zwpaper I think we have a conclusion/consensus 😊 Drop the topologyupdater-job overlay (and related bits e.g. from base)

@zwpaper
Copy link
Member

zwpaper commented Oct 6, 2021

hi @marquiz, I have check docs and other yamls, there is only those two dir containing the topology updater job, I have deleted them, is this all we need to do? it always feels like there is something more I can do.

@marquiz
Copy link
Contributor

marquiz commented Oct 6, 2021

there is only those two dir containing the topology updater job, I have deleted them, is this all we need to do?

Actually, in this case, that should be all as we haven't updated documentation, yet

it always feels like there is something more I can do.

😄 I think it is that simple, this time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Hacktoberfest kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants