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

add prod dirs via .gitkeep #21355

Merged
merged 1 commit into from
Aug 9, 2021
Merged

add prod dirs via .gitkeep #21355

merged 1 commit into from
Aug 9, 2021

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented Jul 28, 2021

part of #20394

the flip side in the rpm is: ManageIQ/manageiq-rpm_build#174

These 3 directories need to be in production (and writeable)

They were created by our rpm build process, but moving the here
to keep the logic in a place that is accessible by both pods and appliances.

@miq-bot
Copy link
Member

miq-bot commented Jul 28, 2021

Checked commit kbrock@2b26458 with ruby 2.6.3, rubocop 1.13.0, haml-lint 0.35.0, and yamllint
0 files checked, 0 offenses detected
Everything looks fine. 🍰

@Fryguy
Copy link
Member

Fryguy commented Jul 28, 2021

Is there an associated issue on the rpm build side to stop creating these directories? Also, is there any source code that tries to do like a mkdir -p to create these directories that can now be removed?

@Fryguy Fryguy self-assigned this Jul 28, 2021
@kbrock kbrock changed the title add prod dirs [WIP] add prod dirs Jul 29, 2021
@miq-bot miq-bot added the wip label Jul 29, 2021
@kbrock
Copy link
Member Author

kbrock commented Jul 29, 2021

created associated PR that no longer mkdir -p
It does chown them so manageiq user can write to them

WIP: verify the rpm properly creates these directories

@agrare
Copy link
Member

agrare commented Aug 4, 2021

Also, is there any source code that tries to do like a mkdir -p to create these directories that can now be removed?

Yes https://github.com/ManageIQ/manageiq-ui-classic/blob/master/app/controllers/ops_controller/settings/common.rb#L6

@kbrock
Copy link
Member Author

kbrock commented Aug 4, 2021

YAY -- good find. we are creating them up front but that is a great example of a directory that should be removed

@agrare
Copy link
Member

agrare commented Aug 4, 2021

we are creating them up front

Actually that one didn't exist on my appliance and the UI worker was trying to create it when I hit the application settings page. (sorry if you were talking about going to be creating them up front)

@kbrock
Copy link
Member Author

kbrock commented Aug 4, 2021

we are creating them up front

Actually that one didn't exist on my appliance and the UI worker was trying to create it when I hit the application settings page. (sorry if you were talking about going to be creating them up front)

Yea, this PR should create those directories with the .gitkeep. Maybe that is a bad assumption, too.

@kbrock
Copy link
Member Author

kbrock commented Aug 4, 2021

I wanted to delete https://github.com/ManageIq/manageiq/blob/master/app/models/git_repository.rb#L226

But was getting failed builds that didn't seem totally logical. ref1 ref2
This may be because of the tests cleaning up after themselves. Not totally sure since one environment worked and the other did not. (the ruby version or processor type should not have affected the way a directory was created by git checkout)

Either way, leaving the mkdir in there. at least for now.

these 3 directories need to be in production (and writeable)
They were created by our rpm build process, but moving that here
to keep it in one place.

This is following suit for what we are doing to create the log directory
@kbrock kbrock changed the title [WIP] add prod dirs add prod dirs via .gitkeep Aug 5, 2021
@kbrock
Copy link
Member Author

kbrock commented Aug 5, 2021

unwip: the dirs show up with correct permissions after an rpm upgrade.

@kbrock kbrock assigned bdunne and unassigned Fryguy Aug 9, 2021
@bdunne bdunne merged commit 7a1fcf8 into ManageIQ:master Aug 9, 2021
@agrare agrare removed the wip label Aug 9, 2021
@kbrock kbrock deleted the more_dirs branch August 9, 2021 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants