OpsMx / spinnaker-helm

Stable helm chart of Spinnaker
Apache License 2.0
23 stars 43 forks source link

Allow no-validate parameter for docker registry #23

Closed ranesagar closed 2 years ago

ranesagar commented 2 years ago

Solves https://github.com/OpsMx/spinnaker-helm/issues/22

From: https://github.com/spinnaker/halyard/blob/master/docs/commands.md#hal-config-provider-docker-registry

--no-validate: (Default: false) Skip validation.

ranesagar commented 2 years ago

@abhinaybyrisetty - Can you please review?

abhinaybyrisetty commented 2 years ago

Yeah, this change looks good. But, skipping validation shouldn't be recommended unless extremely necessary. Perhaps, making client-timeout as configurable is a better option than skipping validation.

Badbond commented 2 years ago

Thanks for picking this up so fast :pray:

Perhaps, making client-timeout as configurable is a better option than skipping validation.

AFAIK, there is no such option. A ticket was created for this but went stale.

I'm afraid these changes won't fully solve the problem as validation will be done during hal deploy apply as well

@abhinaybyrisetty Fortunately this command does have parameters to configure timeout behavior such as --wait-for-completion and --wait-for-completion-timeout-minutes. I believe it also has a longer default timeout (but couldn't find official resources on this). Luckily the chart already allows to pass additional parameters to this command to do so.

abhinaybyrisetty commented 2 years ago

Hi @Badbond

There is an option to pass along with hal config command --client-timeout-millis which will allow us to override timeout. Did you try it?

Badbond commented 2 years ago

Thanks for pointing it out @abhinaybyrisetty, I did not see it before. I did just try it but unfortunately it looks like it is not used during validation (executed in Halyard pod):

$ time hal config provider docker-registry account edit dockerhub --address index.docker.io \ 
    <account settings & --repositories> \
   --client-timeout-millis 600000
+ Get current deployment
  Success
+ Get dockerhub account
  Success
<-^*_ Edit the dockerhub account>
  Timed out
Validation in Global:
! ERROR Unexpected exception:
  DaemonTaskInterrupted(interruptedTime=1652944810613, message=Task interrupted
  without cause at Thu May 19 07:20:10 GMT 2022)

- Failed to edit account dockerhub for provider dockerRegistry.
  Task killed because it was taking too long.

real    1m8.203s
user    0m5.512s
sys 0m0.523s
Badbond commented 2 years ago

@abhinaybyrisetty did you have some time to check out my findings? Is there is anything I can do to help in this PR?

abhinaybyrisetty commented 2 years ago

@Badbond then I think, having the option to skip validation is the more convenient at the moment. Let's submit that.

abhinaybyrisetty commented 2 years ago

@ranesagar I made some changes to the folder structure and added a github action to automate release. You might want to rebase the code.

ranesagar commented 2 years ago

@abhinaybyrisetty - PTAL

Badbond commented 2 years ago

Thanks for rebasing @ranesagar. @abhinaybyrisetty did you have time to check this one out? Anything I can help with?

abhinaybyrisetty commented 2 years ago

LGTM, please bump the chart version to automatically publish the chart to helm repo.

ranesagar commented 2 years ago

@abhinaybyrisetty - PTAL