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

Set *:object_store:enabled in gitlab.yml regardless the value #2579

Merged
merged 1 commit into from
Jul 2, 2022
Merged

Set *:object_store:enabled in gitlab.yml regardless the value #2579

merged 1 commit into from
Jul 2, 2022

Conversation

kkimurak
Copy link
Contributor

@kkimurak kkimurak commented Jun 4, 2022

In current code, upload:object_store:enabled is updated only if GITLAB_UPLOADS_OBJECT_STORE_ENABLED is set to true. It makes puma to die because of invalid value appears to configuration.

This can be fixed by running update_template GITLAB_UPLOADS_OBJECT_STORE_ENABLED regardless of the value.

Here is a /var/log/gitlab/supervisor/puma.log that shows puma reporting exception.

/var/log/gitlab/supervisor/puma.log 
bundler: failed to load command: puma (/home/git/gitlab/vendor/bundle/ruby/2.7.0/bin/puma)
/home/git/gitlab/config/initializers/direct_upload_support.rb:22:in `verify!': Object storage provider '{"{\\"GITLAB_UPLOADS_OBJECT_STORE_CONNECTION_PROVIDER\\"=>nil}"=>nil}' is not supported when 'direct_upload' is used for 'lfs'. Only Google, AWS, and AzureRM are supported. (DirectUploadsValidator::ValidationError)
	from /home/git/gitlab/config/initializers/direct_upload_support.rb:45:in `block (2 levels) in <top (required)>'
	from /home/git/gitlab/config/initializers/direct_upload_support.rb:44:in `each'
	from /home/git/gitlab/config/initializers/direct_upload_support.rb:44:in `block in <top (required)>'
	from /home/git/gitlab/config/initializers/direct_upload_support.rb:37:in `tap'
	from /home/git/gitlab/config/initializers/direct_upload_support.rb:37:in `<top (required)>'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/railties-6.1.4.7/lib/rails/engine.rb:681:in `load'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/railties-6.1.4.7/lib/rails/engine.rb:681:in `block in load_config_initializer'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/activesupport-6.1.4.7/lib/active_support/notifications.rb:205:in `instrument'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/railties-6.1.4.7/lib/rails/engine.rb:680:in `load_config_initializer'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/railties-6.1.4.7/lib/rails/engine.rb:634:in `block (2 levels) in <class:Engine>'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/railties-6.1.4.7/lib/rails/engine.rb:633:in `each'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/railties-6.1.4.7/lib/rails/engine.rb:633:in `block in <class:Engine>'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/railties-6.1.4.7/lib/rails/initializable.rb:32:in `instance_exec'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/railties-6.1.4.7/lib/rails/initializable.rb:32:in `run'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/railties-6.1.4.7/lib/rails/initializable.rb:61:in `block in run_initializers'
	from /usr/local/lib/ruby/2.7.0/tsort.rb:228:in `block in tsort_each'
	from /usr/local/lib/ruby/2.7.0/tsort.rb:350:in `block (2 levels) in each_strongly_connected_component'
	from /usr/local/lib/ruby/2.7.0/tsort.rb:422:in `block (2 levels) in each_strongly_connected_component_from'
	from /usr/local/lib/ruby/2.7.0/tsort.rb:431:in `each_strongly_connected_component_from'
	from /usr/local/lib/ruby/2.7.0/tsort.rb:421:in `block in each_strongly_connected_component_from'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/railties-6.1.4.7/lib/rails/initializable.rb:50:in `each'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/railties-6.1.4.7/lib/rails/initializable.rb:50:in `tsort_each_child'
	from /usr/local/lib/ruby/2.7.0/tsort.rb:415:in `call'
	from /usr/local/lib/ruby/2.7.0/tsort.rb:415:in `each_strongly_connected_component_from'
	from /usr/local/lib/ruby/2.7.0/tsort.rb:349:in `block in each_strongly_connected_component'
	from /usr/local/lib/ruby/2.7.0/tsort.rb:347:in `each'
	from /usr/local/lib/ruby/2.7.0/tsort.rb:347:in `call'
	from /usr/local/lib/ruby/2.7.0/tsort.rb:347:in `each_strongly_connected_component'
	from /usr/local/lib/ruby/2.7.0/tsort.rb:226:in `tsort_each'
	from /usr/local/lib/ruby/2.7.0/tsort.rb:205:in `tsort_each'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/railties-6.1.4.7/lib/rails/initializable.rb:60:in `run_initializers'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/railties-6.1.4.7/lib/rails/application.rb:391:in `initialize!'
	from /home/git/gitlab/config/environment.rb:7:in `<top (required)>'
	from config.ru:5:in `require'
	from config.ru:5:in `block in <main>'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/rack-2.2.3/lib/rack/builder.rb:116:in `eval'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/rack-2.2.3/lib/rack/builder.rb:116:in `new_from_string'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/rack-2.2.3/lib/rack/builder.rb:105:in `load_file'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/rack-2.2.3/lib/rack/builder.rb:66:in `parse_file'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/puma-5.6.2/lib/puma/configuration.rb:348:in `load_rackup'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/puma-5.6.2/lib/puma/configuration.rb:270:in `app'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/puma-5.6.2/lib/puma/runner.rb:150:in `load_and_bind'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/puma-5.6.2/lib/puma/cluster.rb:357:in `run'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/puma-5.6.2/lib/puma/launcher.rb:182:in `run'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/puma-5.6.2/lib/puma/cli.rb:81:in `run'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/gems/puma-5.6.2/bin/puma:10:in `<top (required)>'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/bin/puma:23:in `load'
	from /home/git/gitlab/vendor/bundle/ruby/2.7.0/bin/puma:23:in `<top (required)>'
	from /usr/local/lib/ruby/gems/2.7.0/gems/bundler-2.3.6/lib/bundler/cli/exec.rb:58:in `load'
	from /usr/local/lib/ruby/gems/2.7.0/gems/bundler-2.3.6/lib/bundler/cli/exec.rb:58:in `kernel_load'
	from /usr/local/lib/ruby/gems/2.7.0/gems/bundler-2.3.6/lib/bundler/cli/exec.rb:23:in `run'
	from /usr/local/lib/ruby/gems/2.7.0/gems/bundler-2.3.6/lib/bundler/cli.rb:484:in `exec'
	from /usr/local/lib/ruby/gems/2.7.0/gems/bundler-2.3.6/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
	from /usr/local/lib/ruby/gems/2.7.0/gems/bundler-2.3.6/lib/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command'
	from /usr/local/lib/ruby/gems/2.7.0/gems/bundler-2.3.6/lib/bundler/vendor/thor/lib/thor.rb:392:in `dispatch'
	from /usr/local/lib/ruby/gems/2.7.0/gems/bundler-2.3.6/lib/bundler/cli.rb:31:in `dispatch'
	from /usr/local/lib/ruby/gems/2.7.0/gems/bundler-2.3.6/lib/bundler/vendor/thor/lib/thor/base.rb:485:in `start'
	from /usr/local/lib/ruby/gems/2.7.0/gems/bundler-2.3.6/lib/bundler/cli.rb:25:in `start'
	from /usr/local/lib/ruby/gems/2.7.0/gems/bundler-2.3.6/exe/bundle:48:in `block in <top (required)>'
	from /usr/local/lib/ruby/gems/2.7.0/gems/bundler-2.3.6/lib/bundler/friendly_errors.rb:103:in `with_friendly_errors'
	from /usr/local/lib/ruby/gems/2.7.0/gems/bundler-2.3.6/exe/bundle:36:in `<top (required)>'
	from /usr/local/bin/bundle:23:in `load'
	from /usr/local/bin/bundle:23:in `<main>'

@kkimurak kkimurak mentioned this pull request Jun 4, 2022
Draft
4 tasks
@kkimurak kkimurak force-pushed the set-upload-object_store-enabled-even-if-disabled branch from c6a4ab1 to 21af33f Compare June 7, 2022 10:08
@kkimurak kkimurak changed the title Set upload:object_store:enabled in gitlab.yml even if "false" Set *:object_store:enabled in gitlab.yml regardless the value Jun 7, 2022
@kkimurak
Copy link
Contributor Author

kkimurak commented Jun 7, 2022

I have started the PR only fixing uploads section, but found that can be applied to artifacts, lfs, packages, terraform_state. The log message above shows that the error is happen when initializing lfs:object_store.
I have updated the title.

@kkimurak kkimurak force-pushed the set-upload-object_store-enabled-even-if-disabled branch from 21af33f to ea0b504 Compare June 9, 2022 03:14
@kkimurak kkimurak force-pushed the set-upload-object_store-enabled-even-if-disabled branch from ea0b504 to a70c041 Compare June 16, 2022 03:29
@kkimurak
update *:object_store:enabled regardless of the value
844d57d 
apply for artifact,packages,terraform_state,lfs,uploads
@kkimurak kkimurak force-pushed the set-upload-object_store-enabled-even-if-disabled branch from a70c041 to 844d57d Compare June 20, 2022 23:49
@sachilles
Copy link
Collaborator

@kkimurak I guess your PR can be applied to the branch 14.10.x-stable, isn't it? Moreover, I'd like to ask you to check #2594 to avoid any concurrent PRs.

@sachilles sachilles mentioned this pull request Jun 25, 2022
Merged
@kkimurak
Copy link
Contributor Author

@sachilles Sure it can be applied for 14.x series I think. Use my commit freely if you need to do that.

Then, we'd better to check assignment for whole environment variables. Let's have a discussion at #2594

@kkimurak
Copy link
Contributor Author

Ah sorry for meaningless response above, I misunderstood something.

I have checked #2594 but there were no duplication or conflict. The PR #2594 is about to change default value when the environment variable is not set. I think the PR #2438 author just forgot to change it when copying and modifying the various settings in env-default.
#2579 (this PR) is about to update the key *:object_store:enabled in gitlab.yml regardless its value. This blocks the progress of #2570 (the test succeed if I merg this patch into #2570)

@sachilles sachilles added the backport Label for backports label Jun 25, 2022
@sachilles sachilles mentioned this pull request Jun 25, 2022
Merged
@Syphon83
Copy link
Contributor

Ah sorry for meaningless response above, I misunderstood something.

I have checked #2594 but there were no duplication or conflict. The PR #2594 is about to change default value when the environment variable is not set. I think the PR #2438 author just forgot to change it when copying and modifying the various settings in env-default. #2579 (this PR) is about to update the key *:object_store:enabled in gitlab.yml regardless its value. This blocks the progress of #2570 (the test succeed if I merg this patch into #2570)

Hi @kkimurak, PR #2594 changes the value of the variable even if the variable is set.
I tried first hand and the variable GITLAB_TERRAFORM_STATE_OBJECT_STORE_REMOTE_DIRECTORY is always set as the GITLAB_PACKAGES_OBJECT_STORE_REMOTE_DIRECTORY, even if the terraform one is set.

The result is an overlapping of bucket storage, data that is supposed to go on the terraform state bucket would end up on the packages one.

@kkimurak
Copy link
Contributor Author

Oh sorry. I was completely misunderstood something. You are about to correct assignment of environment value itself, not default value. @Syphon83 Thank you for correcting me.

Anyway, there are no concurrent work. we can merge both with no issues, I think.

@Syphon83
Copy link
Contributor

Oh sorry. I was completely misunderstood something. You are about to correct assignment of environment value itself, not default value. @Syphon83 Thank you for correcting me.

Anyway, there are no concurrent work. we can merge both with no issues, I think.

@kkimurak Don't worry, it was my frustration speaking. I was SO close to make a mess with data inside buckets.
In any case I agree with you. we can merge both PRs and backport them.

@sachilles Do you agree?

Thank you.

@sachilles sachilles merged commit eba7979 into sameersbn:master Jul 2, 2022
@kkimurak kkimurak deleted the set-upload-object_store-enabled-even-if-disabled branch July 2, 2022 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
backport Label for backports
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kkimurak @sachilles @Syphon83