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

fix: fail when use standalone mode (#541) #547

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

fvilla-netnix
Copy link

This aims to fix an existing issue that causes containerized deployments to fail when a config file is provided by the user while in standalone mode.

This is because when in standalone mode the docker-entrypoint.sh script tries to create the config.yaml file or update it's keys to ensure it is setup correctly.

My fix allow users to use read-only files by removing write operations from the script and replaces them with checks and warnings to stop users from using invalid config files.

@kayx23
Copy link
Member

kayx23 commented Mar 12, 2024

Thanks for the PR. Debian standalone CI is failing. Please take a look.

@kayx23
Copy link
Member

kayx23 commented Mar 12, 2024

Hi there, still failing

@fvilla-netnix
Copy link
Author

Think I fixed it now. I had broken some config.yaml files.

@kayx23
Copy link
Member

kayx23 commented Mar 14, 2024

Hi @fvilla-netnix unfortunately tests are still falling. For future reference, it is easier to open PRs to the main branch of your fork to test CIs. This requires no approvals from maintainers and you could get immediate feedback. Since this PR is already opened, I'm not sure if you could still open one to your main branch; likely not.

Comment on lines +24 to +27
if [ "$APISIX_STAND_ALONE" = "true" ]; then
# If the file is not present then initialise the content otherwise check relevant keys for standalone mode
if [ ! -f "${PREFIX}/conf/config.yaml" ]; then
cat > ${PREFIX}/conf/config.yaml << _EOC_
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you change these indents? please leave it as it is, just fix the bug is enough

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I undo those changes? The indentation was driving me crazy in my IDE since it was split between 2 spaces and 4.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please undo those changes, the pr should be as small as possible

@monkeyDluffy6017 monkeyDluffy6017 changed the title Standalone mode config.yaml fix (#541) fix: fail when use standalone mode (#541) Mar 14, 2024
/usr/bin/apisix init
/usr/bin/apisix init_etcd
fi
fi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change the irrelevant indentation changes back for easier review. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants